[PATCH] D33074: InstCombine: Allow sinking instructions with more uses in the same block.

[Kyle Butt via Phabricator via llvm-commits]

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