[PATCH] D113577: [SCEV] Support rewriting ZExt expressions with loop guard info.
Philip Reames via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 12 09:19:43 PST 2021
reames added a comment.
This generally looks very reasonable.
Can I request that you split off the change which switches the map from value keyed to SCEV keyed, and land that without separate review? It seems like it should be entirely NFC. Reducing diff and isolating failures is always good.
================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:13921
+ // sub-expressions.
+ if (RewriteMap.size() > 1) {
+ SmallVector<const SCEV *> ExprsToRewrite;
----------------
This should follow from the empty check just above.
================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:13934
+
SCEVLoopGuardRewriter Rewriter(*this, RewriteMap);
return Rewriter.visit(Expr);
----------------
You can fold this into the previous code by simply pushing Expr onto the replacement list last.
Hm, do we need a fixed point in the loop above? It seems like we have the possibility of one replacement enabling another. Is there something non-obvious in the above which handles that?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113577/new/
https://reviews.llvm.org/D113577
More information about the llvm-commits
mailing list