[llvm] r288851 - [BDCE/DebugInfo] Preserve llvm.dbg.value's argument.
Hal Finkel via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 7 02:23:23 PST 2016
Hi David,
While this will certainly make the situation much better, it does not completely solve the problem. If you have an instruction that may have side effects, has a llvm.dbg.value, also has other uses, and BDCE can show that those other uses don't depend on the value (i.e. there are no demanded bits), then we'll still end up with the same problem. BDCE will end up breaking the value dependency edge between the instruction and the llvm.dbg intrinsic when it breaks the dependency edge with the other users.
We had chatted on IRC about the possibility of creating some RAUWExceptDbg function. You said this got ugly, but maybe we should do something a bit simpler: make a RAUWExceptMetadata function. The implementation of RAUW looks like this:
void Value::replaceAllUsesWith(Value *New) {
...
// Notify all ValueHandles (if present) that this value is going away.
if (HasValueHandle)
ValueHandleBase::ValueIsRAUWd(this, New);
if (isUsedByMetadata())
ValueAsMetadata::handleRAUW(this, New);
while (!use_empty()) {
Use &U = *UseList;
// Must handle Constants specially, we cannot call replaceUsesOfWith on a
// constant because they are uniqued.
if (auto *C = dyn_cast<Constant>(U.getUser())) {
if (!isa<GlobalValue>(C)) {
C->handleOperandChange(this, New);
continue;
}
}
U.set(New);
}
if (BasicBlock *BB = dyn_cast<BasicBlock>(this))
BB->replaceSuccessorsPhiUsesWith(cast<BasicBlock>(New));
}
and we just need a variant that skips the ValueAsMetadata::handleRAUW part. This seems simple enough to arrange (either by adding some optional parameter to the existing implementation or some other refactoring). While we're thinking about it, I'd prefer that we really solve the problem (as opposed to just mostly solve it). I wouldn't revert this change, however, as it is a useful micro-optimization regardless.
Thanks again,
Hal
----- Original Message -----
> From: "Davide Italiano via llvm-commits" <llvm-commits at lists.llvm.org>
> To: llvm-commits at lists.llvm.org
> Sent: Tuesday, December 6, 2016 3:52:47 PM
> Subject: [llvm] r288851 - [BDCE/DebugInfo] Preserve llvm.dbg.value's argument.
>
> Author: davide
> Date: Tue Dec 6 15:52:47 2016
> New Revision: 288851
>
> URL: http://llvm.org/viewvc/llvm-project?rev=288851&view=rev
> Log:
> [BDCE/DebugInfo] Preserve llvm.dbg.value's argument.
>
> BDCE has two phases:
> 1. It asks SimplifyDemandedBits if all the bits of an instruction are
> dead, and if so,
> replaces all its uses with the constant zero.
> 2. Then, it asks SimplifyDemandedBits again if the instruction is
> really dead
> (no side effects etc..) and if so, eliminates it.
>
> Now, in 1) if all the bits of an instruction are dead, we may end up
> replacing a dbg use:
> %call = tail call i32 (...) @g() #4, !dbg !15
> tail call void @llvm.dbg.value(metadata i32 %call, i64 0, metadata
> !8, metadata !16), !dbg !17
> ->
> %call = tail call i32 (...) @g() #4, !dbg !15
> tail call void @llvm.dbg.value(metadata i32 0, i64 0, metadata !8,
> metadata !16), !dbg !17
>
> but not eliminating the call because it may have arbitrary side
> effects.
> In other words, we lose some debug informations.
> This patch fixes the problem making sure that BDCE does nothing with
> the instruction if
> it has side effects and no non-dbg uses.
>
> Differential Revision: https://reviews.llvm.org/D27471
>
> Added:
> llvm/trunk/test/Transforms/BDCE/pr26587.ll
> Modified:
> llvm/trunk/lib/Transforms/Scalar/BDCE.cpp
>
> Modified: llvm/trunk/lib/Transforms/Scalar/BDCE.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/BDCE.cpp?rev=288851&r1=288850&r2=288851&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/BDCE.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/BDCE.cpp Tue Dec 6 15:52:47
> 2016
> @@ -39,6 +39,11 @@ static bool bitTrackingDCE(Function &F,
> SmallVector<Instruction*, 128> Worklist;
> bool Changed = false;
> for (Instruction &I : instructions(F)) {
> + // If the instruction has side effects and no non-dbg uses,
> + // BDCE should skip it.
> + if (I.mayHaveSideEffects() && I.use_empty())
> + continue;
> +
> if (I.getType()->isIntegerTy() &&
> !DB.getDemandedBits(&I).getBoolValue()) {
> // For live instructions that have all dead bits, first make
> them dead by
>
> Added: llvm/trunk/test/Transforms/BDCE/pr26587.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/BDCE/pr26587.ll?rev=288851&view=auto
> ==============================================================================
> --- llvm/trunk/test/Transforms/BDCE/pr26587.ll (added)
> +++ llvm/trunk/test/Transforms/BDCE/pr26587.ll Tue Dec 6 15:52:47
> 2016
> @@ -0,0 +1,46 @@
> +; Test that BDCE doesn't destroy llvm.dbg.value's argument.
> +; RUN: opt -bdce %s -S | FileCheck %s
> +
> +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
> +target triple = "x86_64-unknown-linux-gnu"
> +
> +; CHECK: define void @f()
> +; CHECK-NEXT: entry:
> +; CHECK-NEXT: tail call void (...) @h()
> +; CHECK-NEXT: %[[CALL:.*]] = tail call i32 (...) @g()
> +; CHECK-NEXT: tail call void @llvm.dbg.value(metadata i32
> %[[CALL:.*]]
> +
> +define void @f() !dbg !6 {
> +entry:
> + tail call void (...) @h(), !dbg !9
> + %call = tail call i32 (...) @g(), !dbg !10
> + tail call void @llvm.dbg.value(metadata i32 %call, i64 0, metadata
> !11, metadata !13), !dbg !14
> + tail call void (...) @h(), !dbg !15
> + ret void, !dbg !16
> +}
> +
> +declare void @h(...)
> +declare i32 @g(...)
> +declare void @llvm.dbg.value(metadata, i64, metadata, metadata)
> +
> +!llvm.dbg.cu = !{!0}
> +!llvm.module.flags = !{!3, !4}
> +!llvm.ident = !{!5}
> +
> +!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1,
> producer: "clang version 4.0.0 (trunk 288665) (llvm/trunk 288725)",
> isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug,
> enums: !2)
> +!1 = !DIFile(filename: "patatino.c", directory:
> "/home/davide/work/llvm/build-clang/bin")
> +!2 = !{}
> +!3 = !{i32 2, !"Dwarf Version", i32 4}
> +!4 = !{i32 2, !"Debug Info Version", i32 3}
> +!5 = !{!"clang version 4.0.0 (trunk 288665) (llvm/trunk 288725)"}
> +!6 = distinct !DISubprogram(name: "f", scope: !1, file: !1, line: 3,
> type: !7, isLocal: false, isDefinition: true, scopeLine: 3,
> isOptimized: false, unit: !0, variables: !2)
> +!7 = !DISubroutineType(types: !8)
> +!8 = !{null}
> +!9 = !DILocation(line: 4, column: 3, scope: !6)
> +!10 = !DILocation(line: 5, column: 11, scope: !6)
> +!11 = !DILocalVariable(name: "a", scope: !6, file: !1, line: 5,
> type: !12)
> +!12 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
> +!13 = !DIExpression()
> +!14 = !DILocation(line: 5, column: 7, scope: !6)
> +!15 = !DILocation(line: 6, column: 3, scope: !6)
> +!16 = !DILocation(line: 7, column: 1, scope: !6)
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory
More information about the llvm-commits
mailing list