[PATCH] D56867: [ValueTracking] Early out of isValidAssumeForContext if the assume and the context instruction are the same

[Craig Topper via Phabricator via llvm-commits]

craig.topper created this revision.
craig.topper added reviewers: hfinkel, efriedma.

There are two loops in this function that start at the instruction immediately after the context instruction and scans forward until it encounters the assume or another condition. But if the assume and the context are the same instruction these loops can never find the assume.

Eventually we'll end up in the second loop which will return false when isSafeToSpeculativelyExecute encounters the terminator of the basic block and returns false. But that's seems like a lot of extra looping to get that result.

This was found in asm-goto testing because we failed to add the new CallBr terminator to isSafeToSpeculativelyExecute.


https://reviews.llvm.org/D56867

Files:
  lib/Analysis/ValueTracking.cpp


Index: lib/Analysis/ValueTracking.cpp
===================================================================
--- lib/Analysis/ValueTracking.cpp
+++ lib/Analysis/ValueTracking.cpp
@@ -543,6 +543,11 @@
     return true;
   }
 
+  // If the context instruction is this assume, we shouldn't consider it valid.
+  // Otherwise the loops below will scan forward to the end of the block.
+  if (Inv == CxtI)
+    return false;
+
   // 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())


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D56867.182343.patch
Type: text/x-patch
Size: 614 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190117/0b5d358c/attachment.bin>