[PATCH] D128322: [GuardWidening] Use logical and in widenCondCommon as it stated in doc
Philip Reames via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 22 07:59:18 PDT 2022
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
More information about the llvm-commits
mailing list