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

Craig Topper via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 17 14:33:24 PST 2019


craig.topper marked an inline comment as done.
craig.topper added a comment.

If we return false like we currently do via the loop. Then I don't think it can be tested since its really just making explicit what was previously an odd iteration behavior. If we change it to true then maybe that would be a behavior change we could see.



================
Comment at: lib/Analysis/ValueTracking.cpp:549
+  if (Inv == CxtI)
+    return false;
+
----------------
efriedma wrote:
> I think we should return "true" here?  The condition of an assume should hold at the point the assume executes.
I just did what the existing behavior ended up with when they hit the end of the basic block and called isSafeToSpeculativelyExecute


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

https://reviews.llvm.org/D56867





More information about the llvm-commits mailing list