[PATCH] D121950: [IndVars] Teach replaceCongruentIVs to avoid scrambling induction variables

[Florian Hahn via Phabricator via llvm-commits]

fhahn accepted this revision.
fhahn added a comment.
This revision is now accepted and ready to land.

> However, it can have some side-effects: it isn't aware whether an induction variable is in canonical form, so it can perform replacements which obscure the meaning of the IR.

Agreed, it seems like the restriction should keep loops more analyzable throughout the pipeline. Aggressively removing phis would probably make more sense late in the pipeline/the backend, if it turns out to be needed.

LGTM, but please wait a day or so with committing in case there are additional opinions.



================
Comment at: llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp:2074
+        const SCEV *PhiExpr = SE.getSCEV(Phi);
+        if (isa<SCEVAddRecExpr>(PhiExpr)) {
+          // This phi can be freely truncated to the narrowest phi type. Map the
----------------
Could you add a comment with an explanation why this is restricted here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121950