[PATCH] D128322: [GuardWidening] Use logical and in widenCondCommon as it stated in doc

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

Hm, this adds quite a bit of complexity.

A couple high level thoughts here:

- A good chunk of that complexity comes from the fact that if the original condition involves an and this code expands out to the component checks and then rewrites all at once.  Arguable, we should be able able to emit as LogicalAnd(Cond1, Cond2), and then optimize later, but that's not the way this code is currently structured.  It might be worth thinking about whether we can reorder the optimization step.  (Random thought, not a "please go do this first".)
- The matching logic in this code should probably be extended to handle LogicalAnd.
- Poison can only be introduced in the rewritten condition if a new value reaches the branch.  The range check comparison itself can not produce poison.  Given that, you may be able to simplify this as "identify values from Cond1 which already produce poison", and then "use logical and or freeze only on values in new range checks not in said set".


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128322/new/

https://reviews.llvm.org/D128322