[PATCH] D19912: [SimplifyCFG] isSafeToSpeculateStore now ignores debug info

Chad Rosier via llvm-commits llvm-commits at lists.llvm.org
Wed May 4 07:21:42 PDT 2016


mcrosier added inline comments.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:1451
@@ -1450,3 +1450,3 @@
   // Look for a store to the same pointer in BrBB.
   unsigned MaxNumInstToLookAt = 10;
   for (BasicBlock::reverse_iterator RI = BrBB->rbegin(),
----------------
Henric wrote:
> mcrosier wrote:
> > Actually, in the original code we were doing a pre-decrement prior to the check.  Now the decrement happens after the check, so I believe we're now doing one extra iteration of the loop.
> Well spotted, I'll fix a patch where the initial count is reduced too so the behaviour is unchanged.
Thanks.

================
Comment at: test/Transforms/SimplifyCFG/PR27615-simplify-cond-br.ll:1
@@ +1,2 @@
+; RUN: opt -S -simplifycfg -strip-debug < %s | FileCheck --check-prefix CHECK-NO-DBG %s
+; RUN: opt -S -simplifycfg < %s | FileCheck --check-prefix CHECK-DBG %s
----------------
Henric wrote:
> mcrosier wrote:
> > If the expectation is that the output should be exactly the same with and without debug, I'd suggest dropping the prefixes from both RUN commands and have only a single version of the check.  I.e.,
> > 
> >   ; CHECK: select i1 %or.cond, i32 -1, i32 5
> >   ; CHECK-NOT: bb1:
> The reason why I made different checks, was to make it easier to see which run that failed. But maybe it's better to see that the tests are actually equal.
I'd prefer a single check, but I'll let you make the call as it makes very little differences and is a subjective call.


http://reviews.llvm.org/D19912





More information about the llvm-commits mailing list