[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