[PATCH] D113293: [SimplifyCFG] Add early bailout if Use is not in same BB.

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D113293#3114654 <https://reviews.llvm.org/D113293#3114654>, @mkazantsev wrote:

> I don't understand the old code. Why does it look at one user? I thought the order of uses is undefined, and whatever checks are done for just one user and not done for the others may be a source of non-determinism.

The order is deterministic, but usually arbitrary (see `uselistorder` in IR and `-preserve-(bc|ll)-uselistorder`).



================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:6530
     // Only look at the first use, avoid hurting compile time with long uselists
-    User *Use = *I->user_begin();
+    auto *Use = dyn_cast<Instruction>(*I->user_begin());
+    // Bail out if Use is not in the same BB as I or Use == I or Use comes
----------------
Can be `cast`, user of instruction is always instruction.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:6544
+          return !isGuaranteedToTransferExecutionToSuccessor(&I);
+        }))
+      return false;
----------------
We have isGuaranteedToTransferExecutionToSuccessor() variants that accept iterators in ValueTracking, and also enforce a ScanLimit (default 32). That should ensure that there are no pathological cases even if they are in the same block.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113293