[PATCH] Fixing a heisenbug where the memory dependence analysis behaves differently with and without -g

[David Blaikie]

Idea looks sound - could use a commitable test case, though.


On Mon, Nov 11, 2013 at 7:36 PM, Yunzhong Gao <
Yunzhong_Gao at playstation.sony.com> wrote:

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