[PATCH] D33074: InstCombine: Allow sinking instructions with more uses in the same block.
Kyle Butt via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu May 11 07:41:50 PDT 2017
iteratee added inline comments.
================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:2920
+ UsesInOneBlock = true;
+ } else if (CurrentUserParent != UsersParent) {
+ UsesInOneBlock = false;
----------------
dberris wrote:
> Do you actually need the else here? I suspect CurrentUserParent will never be nullptr, and UsersParent will never be nullptr here either after the preceding conditional.
I think it's conceptually clearer with the else. First run vs following runs. It's probably correct either way.
================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:2927
+
+ if (UsesInOneBlock) {
+ BasicBlock *BB = I->getParent();
----------------
dberris wrote:
> Maybe clearer if you turn this into an early return?
>
> ```
> if (!UsesInOneBlock)
> return false;
>
> // rest of code.
> ```
Clearer, but incorrect, there are additional things below.
Repository:
rL LLVM
https://reviews.llvm.org/D33074
More information about the llvm-commits
mailing list