[PATCH] D90456: [IndVars] Use more precise context when eliminating narrowing

Serguei Katkov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 17 23:02:00 PST 2020


skatkov added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyIndVar.cpp:1564
+       Context = User;
+     else if (DT->dominates(User, Context))
+       Context = User;
----------------
anna wrote:
> dominates is a more expensive check than `comesBefore`. We can special case for uses in the same basic block, i.e. :
> ```
> if (User->getParent() == Context->getParent() && User->comesBefore(Context))
>     Context = User
> ```
comesBefore is used in DT->dominates...


================
Comment at: llvm/lib/Transforms/Utils/SimplifyIndVar.cpp:1569
+       Context =
+           DT->findNearestCommonDominator(Context->getParent(),
+                                          User->getParent())->getTerminator();
----------------
What do you think about introducing the method in DT which finds nearestCommonDominator for an array of values?
Here, just to collect all users and trigger the utility method?

Alternatively, introduce findNearestCommonDominator which accepts Instructions or values in DomTree?
And use just it instead of this dispatching.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyIndVar.cpp:1573
+  // No users, the instruction is dead.
+  if (!Context) {
+    DeadInsts.emplace_back(NarrowUse);
----------------
If all users are NarrowDef, should we mark it as dead?


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

https://reviews.llvm.org/D90456



More information about the llvm-commits mailing list