[PATCH] D113577: [SCEV] Support rewriting ZExt expressions with loop guard info.

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked an inline comment as done.
fhahn added inline comments.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:13725
+    return I->second;
+  }
 };
----------------
nikic wrote:
> Can you please add a test where you both have a condition on %x and on `zext %x`? I think we'll end up losing the information from the direct condition in that case.
> 
> Something I also find odd here is the mismatch between the code collecting rewrites and the visitor. We'll collect rewrites on all SCEVs, but then only rewrite unknown and zext.
> Can you please add a test where you both have a condition on %x and on zext %x? I think we'll end up losing the information from the direct condition in that case.

Added those tests in 5dfe60d171d7. The original version failed to apply information of sub-expressions to larger expressions, so we failed to determine the tightest bound in one the cases. I updated the patch to re-write the collected expressions with the information we collected, which should resolve that issue.

> Something I also find odd here is the mismatch between the code collecting rewrites and the visitor. We'll collect rewrites on all SCEVs, but then only rewrite unknown and zext.

Good point! There are other motivating cases that require re-writing other expressions as well (for PR40961), but for now I also restricted collecting to LHSs that are either `SCEVUnknown` or `SCEVZeroExtendExpr`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113577