<div dir="ltr">Idea looks sound - could use a commitable test case, though.</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Nov 11, 2013 at 7:36 PM, Yunzhong Gao <span dir="ltr"><<a href="mailto:Yunzhong_Gao@playstation.sony.com" target="_blank">Yunzhong_Gao@playstation.sony.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">ygao added you to the CC list for the revision "Fixing a heisenbug where the memory dependence analysis behaves differently with and without -g".<br>

<br>
Hi,<br>
I was debugging the following cpp file and I noticed that the memory dependence analysis returns<br>
different results depending on whether the LLVM IR file contains debug metadata. I traced it down<br>
to a place in the memory dependence analysis pass where the debug intrinsics are counted into<br>
the permitted number of instructions to be scanned before bailing out.<br>
<br>
Here is the test case:<br>
```<br>
/* test.cpp<br>
 *<br>
 * To reproduce:<br>
 * (1) compile with -g (test.cpp => O1g.ll => O1g_o.ll => O1g.s)<br>
 *  $ clang++ -cc1 -triple x86_64-unknown-linux-gnu -emit-llvm \<br>
 *    -mrelocation-model static -target-cpu x86-64 -std=gnu++0x -O1 -g \<br>
 *    -o O1g.ll -x c++ test.cpp<br>
 *  $ opt -tbaa -basicaa -inline -sroa -early-cse -simplifycfg -loop-unroll \<br>
 *    -gvn -instcombine -dse O1g.ll -S -o O1g_o.ll<br>
 *  $ llc -march=x86-64 -mtriple=x86_64-unknown-linux-gnu \<br>
 *    -relocation-model=static -filetype=asm O1g_o.ll -o O1g.s<br>
 *<br>
 * (2) compile without -g (test.cpp => O1.ll => O1_o.ll => O1.s)<br>
 *  $ clang++ -cc1 -triple x86_64-unknown-linux-gnu -emit-llvm \<br>
 *    -mrelocation-model static -target-cpu x86-64 -std=gnu++0x -O1 \<br>
 *    -o O1.ll -x c++ test.cpp<br>
 *  $ opt -tbaa -basicaa -inline -sroa -early-cse -simplifycfg -loop-unroll \<br>
 *    -gvn -instcombine -dse O1.ll -S -o O1_o.ll<br>
 *  $ llc -march=x86-64 -mtriple=x86_64-unknown-linux-gnu \<br>
 *    -relocation-model=static -filetype=asm O1_o.ll -o O1.s<br>
 *<br>
 * (3) compare the outputs O1g.s and O1.s; they have different instructions.<br>
 *<br>
 */<br>
<br>
typedef unsigned long size_t;<br>
<br>
inline void *operator new(size_t, void *where) { return where; }<br>
<br>
struct int_array<br>
{<br>
  int_array(int *input, size_t count)<br>
  {<br>
    data = (int *) ::operator new(count * sizeof(int));<br>
    for (size_t idx = 0; idx < count; ++idx, ++input)<br>
      ::new (&data[idx]) int(static_cast<int&&>(*input));<br>
  }<br>
  ~int_array() noexcept { delete data; data = 0; }<br>
<br>
  int& operator[](size_t idx) { return data[idx]; }<br>
<br>
private:<br>
  int *data;<br>
};<br>
<br>
int main()<br>
{<br>
    int a1[16] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15};<br>
    int a2[5]  = {-1, -2, -3, -4, -5};<br>
<br>
    int_array v1(a1, 16);<br>
    int_array v2(a2, 5);<br>
<br>
    v1[1]  = -1;<br>
    v1[4]  = -2;<br>
    v1[7]  = -3;<br>
    v1[10] = -4;<br>
    v1[13] = -5;<br>
<br>
    return 0;<br>
}<br>
/* end of test.cpp */<br>
```<br>
<br>
The patch is fairly straight-forward in itself, although I am seeking some advice on how to<br>
write a regression test for this bug. Or should I just submit the patch without a test case?<br>
<br>
- Gao.<br>
<br>
<a href="http://llvm-reviews.chandlerc.com/D2141" target="_blank">http://llvm-reviews.chandlerc.com/D2141</a><br>
<br>
Files:<br>
  llvm/lib/Analysis/MemoryDependenceAnalysis.cpp<br>
<br>
Index: llvm/lib/Analysis/MemoryDependenceAnalysis.cpp<br>
===================================================================<br>
--- llvm/lib/Analysis/MemoryDependenceAnalysis.cpp<br>
+++ llvm/lib/Analysis/MemoryDependenceAnalysis.cpp<br>
@@ -371,18 +371,19 @@<br>
<br>
   // Walk backwards through the basic block, looking for dependencies.<br>
   while (ScanIt != BB->begin()) {<br>
+    Instruction *Inst = --ScanIt;<br>
+<br>
+    if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(Inst))<br>
+      // Debug intrinsics don't (and can't) cause dependences.<br>
+      if (isa<DbgInfoIntrinsic>(II)) continue;<br>
+<br>
     // Limit the amount of scanning we do so we don't end up with quadratic<br>
     // running time on extreme testcases.<br>
     --Limit;<br>
     if (!Limit)<br>
       return MemDepResult::getUnknown();<br>
<br>
-    Instruction *Inst = --ScanIt;<br>
-<br>
     if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(Inst)) {<br>
-      // Debug intrinsics don't (and can't) cause dependences.<br>
-      if (isa<DbgInfoIntrinsic>(II)) continue;<br>
-<br>
       // If we reach a lifetime begin or end marker, then the query ends here<br>
       // because the value is undefined.<br>
       if (II->getIntrinsicID() == Intrinsic::lifetime_start) {<br>
</blockquote></div><br></div>