[PATCH] D121585: [InstCombine] Sink instructions with multiple users in a successor block.
weiwei via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 15 07:59:45 PDT 2022
wwei added inline comments.
================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:4028
return None;
- auto *UserInst = cast_or_null<Instruction>(I->getUniqueUndroppableUser());
- if (!UserInst)
+ if (!I->hasNUndroppableUsesOrMore(1))
return None;
----------------
reames wrote:
> I don't think you need this check at all. You can simple check that UniqueParent is non-null after the loop as that implies there must be at least one use.
check removed
================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:4033
BasicBlock *UserParent = nullptr;
+ BasicBlock *UniqueParent = nullptr;
----------------
nikic wrote:
> The mixed usage of UserParent and UniqueParent here is odd. You'll want to move UserParent inside the loop and only use one variable outside it.
UniqueParent removed
================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:4062
// not bother. SimplifyCFG should handle it.
if (UserParent == BB || !DT.isReachableFromEntry(UserParent))
return None;
----------------
nikic wrote:
> It might make sense to move these checks to the first time we populate the user block, so we e.g. don't waste time visiting all users of an instruction in the same block.
Thanks, these checks moved into for loop
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121585/new/
https://reviews.llvm.org/D121585
More information about the llvm-commits
mailing list