[PATCH] D76228: [ValueTracking] Use Inst::comesBefore in isValidAssumeForCtx (NFC).
Florian Hahn via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 1 04:24:54 PDT 2020
fhahn updated this revision to Diff 254160.
fhahn added a comment.
> Generally, I think this code is a bit convoluted, and I think it would be clearer to rewrite along these lines.
Agreed, I've restructured the code a bit. It seemed a bit more straight forward to check and handle the case were both instructions are in the same BB up front, followed by the DT check. WDYT?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76228/new/
https://reviews.llvm.org/D76228
Files:
llvm/lib/Analysis/ValueTracking.cpp
Index: llvm/lib/Analysis/ValueTracking.cpp
===================================================================
--- llvm/lib/Analysis/ValueTracking.cpp
+++ llvm/lib/Analysis/ValueTracking.cpp
@@ -617,6 +617,29 @@
// feeding the assume is trivially true, thus causing the removal of
// the assume).
+ if (Inv->getParent() == CxtI->getParent()) {
+ // If Inv and CtxI are in the same block, check if the assume (Inv) is first
+ // in the BB.
+ if (Inv->comesBefore(CxtI))
+ return true;
+
+ // Don't let an assume affect itself - this would cause the problems
+ // `isEphemeralValueOf` is trying to prevent, and it would also make
+ // the loop below go out of bounds.
+ if (Inv == CxtI)
+ return false;
+
+ // The context comes first, but they're both in the same block.
+ // Make sure there is nothing in between that might interrupt
+ // the control flow, not even CxtI itself.
+ for (BasicBlock::const_iterator I(CxtI), IE(Inv); I != IE; ++I)
+ if (!isGuaranteedToTransferExecutionToSuccessor(&*I))
+ return false;
+
+ return !isEphemeralValueOf(Inv, CxtI);
+ }
+
+ // Inv and CxtI are in different blocks.
if (DT) {
if (DT->dominates(Inv, CxtI))
return true;
@@ -625,37 +648,7 @@
return true;
}
- // With or without a DT, the only remaining case we will check is if the
- // instructions are in the same BB. Give up if that is not the case.
- if (Inv->getParent() != CxtI->getParent())
- return false;
-
- // If we have a dom tree, then we now know that the assume doesn't dominate
- // the other instruction. If we don't have a dom tree then we can check if
- // the assume is first in the BB.
- if (!DT) {
- // Search forward from the assume until we reach the context (or the end
- // of the block); the common case is that the assume will come first.
- for (auto I = std::next(BasicBlock::const_iterator(Inv)),
- IE = Inv->getParent()->end(); I != IE; ++I)
- if (&*I == CxtI)
- return true;
- }
-
- // Don't let an assume affect itself - this would cause the problems
- // `isEphemeralValueOf` is trying to prevent, and it would also make
- // the loop below go out of bounds.
- if (Inv == CxtI)
- return false;
-
- // The context comes first, but they're both in the same block.
- // Make sure there is nothing in between that might interrupt
- // the control flow, not even CxtI itself.
- for (BasicBlock::const_iterator I(CxtI), IE(Inv); I != IE; ++I)
- if (!isGuaranteedToTransferExecutionToSuccessor(&*I))
- return false;
-
- return !isEphemeralValueOf(Inv, CxtI);
+ return false;
}
static bool isKnownNonZeroFromAssume(const Value *V, const Query &Q) {
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D76228.254160.patch
Type: text/x-patch
Size: 2734 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200401/32508fea/attachment.bin>
More information about the llvm-commits
mailing list