[PATCH] Fixing a heisenbug where the memory dependence analysis behaves differently with and without -g
David Blaikie
dblaikie at gmail.com
Tue Nov 12 14:26:04 PST 2013
On Tue, Nov 12, 2013 at 2:23 PM, Yunzhong Gao <
Yunzhong_Gao at playstation.sony.com> wrote:
> Hi David,
> Thanks for taking a look at the patch, that is really quick action!
> Do you have some suggestions on how a test for this kind of bug should
> look like? I could not find a way to directly test this part of
> memdep analysis pass; my best attempt so far is to trick an optimization
> pass that depends on the outputs of the memdep analysis.
> For example, since the current limit in the memory dependence analysis
> is 100, I could insert 100 dbg.value intrinsics between two
> stores and check that the dead store elimination still takes place.
> There are several down sides to my approach. Other than that it
> is not a direct test, if the threshold in the memdep pass increases
> again in the future, the test would become ineffective (before
> r179713, it would take 500 dbg.value intrinsics to expose this bug).
>
Are there existing test cases that ensure the threshold is respected? If
you massively increase or decrease the threshold, do any tests fail? If
not, then it seems we're missing test coverage. We should have some tests
that check that, and then we could have another variant that has a trash
llvm.dbg.value intrinsic designed to push the block over the limit and
ensure that we get the same behavior as if the intrinsic is not present.
- David
> - Gao.
>
> http://llvm-reviews.chandlerc.com/D2141
>
> CHANGE SINCE LAST DIFF
> http://llvm-reviews.chandlerc.com/D2141?vs=5448&id=5480#toc
>
> Files:
> test/Transforms/DeadStoreElimination/dbg_intrinsics.ll
> lib/Analysis/MemoryDependenceAnalysis.cpp
>
> Index: test/Transforms/DeadStoreElimination/dbg_intrinsics.ll
> ===================================================================
> --- test/Transforms/DeadStoreElimination/dbg_intrinsics.ll
> +++ test/Transforms/DeadStoreElimination/dbg_intrinsics.ll
> @@ -0,0 +1,144 @@
> +; RUN: opt -S -dse < %s | FileCheck %s
> +
> +; If there are two stores to the same location, DSE should be able to
> remove
> +; the first store regardless of debug intrinsics between the stores.
> +
> + at x = global i32 0, align 4
> +
> +; Function Attrs: nounwind
> +define i32 @main() {
> +entry:
> + ; The first store; later there is a second store to the same location,
> + ; so this store should be optimized away by DSE.
> + ; CHECK-NOT: store i32 1, i32* @x, align 4
> + store i32 1, i32* @x, align 4
> +
> + ; Insert 100 meaningless dbg.value intrinsics between the two stores
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> + call void @llvm.dbg.value(metadata !12, i64 0, metadata !10)
> +
> + ; CHECK: store i32 -1, i32* @x, align 4
> + store i32 -1, i32* @x, align 4
> + ret i32 0
> +}
> +
> +; Function Attrs: nounwind readnone
> +declare void @llvm.dbg.value(metadata, i64, metadata)
> +
> +!llvm.dbg.cu = !{!0}
> +!llvm.module.flags = !{!11}
> +
> +!0 = metadata !{i32 786449, metadata !1, i32 4, metadata !"clang version
> 3.4", i1 true, metadata !"", i32 0, metadata !2, metadata !2, metadata !3,
> metadata !9, metadata !2, metadata !""} ; [ DW_TAG_compile_unit ]
> [/home/tmp/test.c] [DW_LANG_C99]
> +!1 = metadata !{metadata !"test.c", metadata !"/home/tmp"}
> +!2 = metadata !{i32 0}
> +!3 = metadata !{metadata !4}
> +!4 = metadata !{i32 786478, metadata !1, metadata !5, metadata !"main",
> metadata !"main", metadata !"", i32 3, metadata !6, i1 false, i1 true, i32
> 0, i32 0, null, i32 256, i1 false, i32 ()* @main, null, null, metadata !2,
> i32 4} ; [ DW_TAG_subprogram ] [line 3] [def] [scope 4] [main]
> +!5 = metadata !{i32 786473, metadata !1} ; [ DW_TAG_file_type ]
> [/home/tmp/test.c]
> +!6 = metadata !{i32 786453, i32 0, null, metadata !"", i32 0, i64 0, i64
> 0, i64 0, i32 0, null, metadata !7, i32 0, null, null, null} ; [
> DW_TAG_subroutine_type ] [line 0, size 0, align 0, offset 0] [from ]
> +!7 = metadata !{metadata !8}
> +!8 = metadata !{i32 786468, null, null, metadata !"int", i32 0, i64 32,
> i64 32, i64 0, i32 0, i32 5} ; [ DW_TAG_base_type ] [int] [line 0, size 32,
> align 32, offset 0, enc DW_ATE_signed]
> +!9 = metadata !{metadata !10}
> +!10 = metadata !{i32 786484, i32 0, null, metadata !"x", metadata !"x",
> metadata !"", metadata !5, i32 1, metadata !8, i32 0, i32 1, i32* @x, null}
> ; [ DW_TAG_variable ] [x] [line 1] [def]
> +!11 = metadata !{i32 2, metadata !"Dwarf Version", i32 4}
> +!12 = metadata !{i32* undef}
> +
> Index: lib/Analysis/MemoryDependenceAnalysis.cpp
> ===================================================================
> --- lib/Analysis/MemoryDependenceAnalysis.cpp
> +++ 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/20131112/932c8ff3/attachment.html>
More information about the llvm-commits
mailing list