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

[Henric Karlsson via llvm-commits]

Henric 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(),
----------------
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.

================
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
----------------
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.


http://reviews.llvm.org/D19912