[llvm] r334830 - Re-apply "[DebugInfo] Check size of variable in ConvertDebugDeclareToDebugValue"
Bjorn Pettersson via llvm-commits
llvm-commits at lists.llvm.org
Fri Jun 15 06:48:55 PDT 2018
Author: bjope
Date: Fri Jun 15 06:48:55 2018
New Revision: 334830
URL: http://llvm.org/viewvc/llvm-project?rev=334830&view=rev
Log:
Re-apply "[DebugInfo] Check size of variable in ConvertDebugDeclareToDebugValue"
This is r334704 (which was reverted in r334732) with a fix for
types like x86_fp80. We need to use getTypeAllocSizeInBits and
not getTypeStoreSizeInBits to avoid dropping debug info for
such types.
Original commit msg:
> Summary:
> Do not convert a DbgDeclare to DbgValue if the store
> instruction only refer to a fragment of the variable
> described by the DbgDeclare.
>
> Problem was seen when for example having an alloca for an
> array or struct, and there were stores to individual elements.
> In the past we inserted a DbgValue intrinsics for each store,
> just as if the store wrote the whole variable.
>
> When handling store instructions we insert a DbgValue that
> indicates that the variable is "undefined", as we do not know
> which part of the variable that is updated by the store.
>
> When ConvertDebugDeclareToDebugValue is used with a load/phi
> instruction we assert that the referenced value is large enough
> to cover the whole variable. Afaict this should be true for all
> scenarios where those methods are used on trunk. If the assert
> blows in the future I guess we could simply skip to insert a
> dbg.value instruction.
>
> In the future I think we should examine which part of the variable
> that is accessed, and add a DbgValue instrinsic with an appropriate
> DW_OP_LLVM_fragment expression.
>
> Reviewers: dblaikie, aprantl, rnk
>
> Reviewed By: aprantl
>
> Subscribers: JDevlieghere, llvm-commits
>
> Tags: #debug-info
>
> Differential Revision: https://reviews.llvm.org/D48024
Added:
llvm/trunk/test/DebugInfo/X86/mem2reg_fp80.ll
llvm/trunk/test/Transforms/Mem2Reg/debug-alloca-phi-2.ll
Modified:
llvm/trunk/include/llvm/IR/IntrinsicInst.h
llvm/trunk/lib/IR/IntrinsicInst.cpp
llvm/trunk/lib/Transforms/Utils/Local.cpp
llvm/trunk/test/Transforms/InstCombine/debuginfo.ll
Modified: llvm/trunk/include/llvm/IR/IntrinsicInst.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/IntrinsicInst.h?rev=334830&r1=334829&r2=334830&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/IntrinsicInst.h (original)
+++ llvm/trunk/include/llvm/IR/IntrinsicInst.h Fri Jun 15 06:48:55 2018
@@ -93,6 +93,10 @@ namespace llvm {
return cast<MetadataAsValue>(getArgOperand(2))->getMetadata();
}
+ /// Get the size (in bits) of the variable, or fragment of the variable that
+ /// is described.
+ Optional<uint64_t> getFragmentSizeInBits() const;
+
/// \name Casting methods
/// @{
static bool classof(const IntrinsicInst *I) {
Modified: llvm/trunk/lib/IR/IntrinsicInst.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/IntrinsicInst.cpp?rev=334830&r1=334829&r2=334830&view=diff
==============================================================================
--- llvm/trunk/lib/IR/IntrinsicInst.cpp (original)
+++ llvm/trunk/lib/IR/IntrinsicInst.cpp Fri Jun 15 06:48:55 2018
@@ -24,6 +24,7 @@
#include "llvm/IR/IntrinsicInst.h"
#include "llvm/ADT/StringSwitch.h"
#include "llvm/IR/Constants.h"
+#include "llvm/IR/DebugInfoMetadata.h"
#include "llvm/IR/GlobalVariable.h"
#include "llvm/IR/Metadata.h"
#include "llvm/IR/Module.h"
@@ -48,6 +49,12 @@ Value *DbgInfoIntrinsic::getVariableLoca
return nullptr;
}
+Optional<uint64_t> DbgInfoIntrinsic::getFragmentSizeInBits() const {
+ if (auto Fragment = getExpression()->getFragmentInfo())
+ return Fragment->SizeInBits;
+ return getVariable()->getSizeInBits();
+}
+
int llvm::Intrinsic::lookupLLVMIntrinsicByName(ArrayRef<const char *> NameTable,
StringRef Name) {
assert(Name.startswith("llvm."));
Modified: llvm/trunk/lib/Transforms/Utils/Local.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/Local.cpp?rev=334830&r1=334829&r2=334830&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/Local.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/Local.cpp Fri Jun 15 06:48:55 2018
@@ -1228,6 +1228,23 @@ static bool PhiHasDebugValue(DILocalVari
return false;
}
+/// Check if the alloc size of \p ValTy is large enough to cover the variable
+/// (or fragment of the variable) described by \p DII.
+///
+/// This is primarily intended as a helper for the different
+/// ConvertDebugDeclareToDebugValue functions. The dbg.declare/dbg.addr that is
+/// converted describes an alloca'd variable, so we need to use the
+/// alloc size of the value when doing the comparison. E.g. an i1 value will be
+/// identified as covering an n-bit fragment, if the store size of i1 is at
+/// least n bits.
+static bool valueCoversEntireFragment(Type *ValTy, DbgInfoIntrinsic *DII) {
+ const DataLayout &DL = DII->getModule()->getDataLayout();
+ uint64_t ValueSize = DL.getTypeAllocSizeInBits(ValTy);
+ if (auto FragmentSize = DII->getFragmentSizeInBits())
+ return ValueSize >= *FragmentSize;
+ return false;
+}
+
/// Inserts a llvm.dbg.value intrinsic before a store to an alloca'd value
/// that has an associated llvm.dbg.declare or llvm.dbg.addr intrinsic.
void llvm::ConvertDebugDeclareToDebugValue(DbgInfoIntrinsic *DII,
@@ -1238,6 +1255,21 @@ void llvm::ConvertDebugDeclareToDebugVal
auto *DIExpr = DII->getExpression();
Value *DV = SI->getOperand(0);
+ if (!valueCoversEntireFragment(SI->getValueOperand()->getType(), DII)) {
+ // FIXME: If storing to a part of the variable described by the dbg.declare,
+ // then we want to insert a dbg.value for the corresponding fragment.
+ LLVM_DEBUG(dbgs() << "Failed to convert dbg.declare to dbg.value: "
+ << *DII << '\n');
+ // For now, when there is a store to parts of the variable (but we do not
+ // know which part) we insert an dbg.value instrinsic to indicate that we
+ // know nothing about the variable's content.
+ DV = UndefValue::get(DV->getType());
+ if (!LdStHasDebugValue(DIVar, DIExpr, SI))
+ Builder.insertDbgValueIntrinsic(DV, DIVar, DIExpr, DII->getDebugLoc(),
+ SI);
+ return;
+ }
+
// If an argument is zero extended then use argument directly. The ZExt
// may be zapped by an optimization pass in future.
Argument *ExtendedArg = nullptr;
@@ -1281,6 +1313,9 @@ void llvm::ConvertDebugDeclareToDebugVal
if (LdStHasDebugValue(DIVar, DIExpr, LI))
return;
+ assert(valueCoversEntireFragment(LI->getType(), DII) &&
+ "Load is not loading the full variable fragment.");
+
// We are now tracking the loaded value instead of the address. In the
// future if multi-location support is added to the IR, it might be
// preferable to keep tracking both the loaded value and the original
@@ -1301,6 +1336,9 @@ void llvm::ConvertDebugDeclareToDebugVal
if (PhiHasDebugValue(DIVar, DIExpr, APN))
return;
+ assert(valueCoversEntireFragment(APN->getType(), DII) &&
+ "PHI node is not describing the full variable.");
+
BasicBlock *BB = APN->getParent();
auto InsertionPt = BB->getFirstInsertionPt();
Added: llvm/trunk/test/DebugInfo/X86/mem2reg_fp80.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/X86/mem2reg_fp80.ll?rev=334830&view=auto
==============================================================================
--- llvm/trunk/test/DebugInfo/X86/mem2reg_fp80.ll (added)
+++ llvm/trunk/test/DebugInfo/X86/mem2reg_fp80.ll Fri Jun 15 06:48:55 2018
@@ -0,0 +1,57 @@
+; RUN: opt < %s -mem2reg -S | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define dso_local x86_fp80 @powixf2() !dbg !1 {
+entry:
+ %r = alloca x86_fp80, align 16
+ call void @llvm.dbg.declare(metadata x86_fp80* %r, metadata !14, metadata !DIExpression()), !dbg !15
+ br i1 undef, label %if.then, label %if.end, !dbg !16
+
+if.then: ; preds = %entry
+; CHECK-label: if.then:
+; CHECK: %mul = fmul x86_fp80
+; CHECK: call void @llvm.dbg.value(metadata x86_fp80 %mul, metadata {{.*}}, metadata !DIExpression())
+ %mul = fmul x86_fp80 undef, undef, !dbg !18
+ store x86_fp80 %mul, x86_fp80* %r, align 16, !dbg !18
+ br label %if.end, !dbg !20
+
+if.end: ; preds = %if.then, %entry
+; CHECK-label: if.end:
+; CHECK: %r.0 = phi x86_fp80
+; CHECK: call void @llvm.dbg.value(metadata x86_fp80 %r.0, metadata {{.*}}, metadata !DIExpression())
+ %out = load x86_fp80, x86_fp80* %r, align 16, !dbg !21
+ ret x86_fp80 %out, !dbg !22
+}
+
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #0
+
+attributes #0 = { nounwind readnone speculatable }
+
+!llvm.dbg.cu = !{}
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 2, !"Debug Info Version", i32 3}
+!1 = distinct !DISubprogram(name: "__powixf2", scope: !2, file: !2, line: 22, type: !3, isLocal: false, isDefinition: true, scopeLine: 23, flags: DIFlagPrototyped, isOptimized: true, unit: !11, retainedNodes: !13)
+!2 = !DIFile(filename: "powixf2.c", directory: "")
+!3 = !DISubroutineType(types: !4)
+!4 = !{!5, !5, !6}
+!5 = !DIBasicType(name: "long double", size: 128, encoding: DW_ATE_float)
+!6 = !DIDerivedType(tag: DW_TAG_typedef, name: "si_int", file: !7, line: 28, baseType: !8)
+!7 = !DIFile(filename: "int_types.h", directory: "")
+!8 = !DIDerivedType(tag: DW_TAG_typedef, name: "int32_t", file: !9, line: 39, baseType: !10)
+!9 = !DIFile(filename: "/usr/include/stdint.h", directory: "")
+!10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!11 = distinct !DICompileUnit(language: DW_LANG_C99, file: !2, producer: "clang version 7.0.0 () ()", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !12)
+!12 = !{}
+!13 = !{!14}
+!14 = !DILocalVariable(name: "r", scope: !1, file: !2, line: 25, type: !5)
+!15 = !DILocation(line: 25, column: 17, scope: !1)
+!16 = !DILocation(line: 28, column: 13, scope: !17)
+!17 = distinct !DILexicalBlock(scope: !1, file: !2, line: 27, column: 5)
+!18 = !DILocation(line: 29, column: 15, scope: !19)
+!19 = distinct !DILexicalBlock(scope: !17, file: !2, line: 28, column: 13)
+!20 = !DILocation(line: 29, column: 13, scope: !19)
+!21 = !DILocation(line: 35, column: 22, scope: !1)
+!22 = !DILocation(line: 35, column: 5, scope: !1)
Modified: llvm/trunk/test/Transforms/InstCombine/debuginfo.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/debuginfo.ll?rev=334830&r1=334829&r2=334830&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/debuginfo.ll (original)
+++ llvm/trunk/test/Transforms/InstCombine/debuginfo.ll Fri Jun 15 06:48:55 2018
@@ -71,9 +71,11 @@ entry:
; NOLOWER-NOT: alloca
; NOLOWER-NOT: store
; NOLOWER-NOT: call void @llvm.dbg.declare
-; NOLOWER: call void @llvm.dbg.value(metadata i64 %o.coerce0, {{.*}})
+; Here we want to find: call void @llvm.dbg.value(metadata i64 %o.coerce0, metadata [[VARIABLE_O]], metadata !DIExpression(DW_OP_LLVM_fragment, 0, 64))
+; NOLOWER: call void @llvm.dbg.value(metadata i64 undef, {{.*}})
; NOLOWER-NOT: store
-; NOLOWER: call void @llvm.dbg.value(metadata i64 %o.coerce1, {{.*}})
+; Here we want to find: call void @llvm.dbg.value(metadata i64 %o.coerce1, metadata [[VARIABLE_O]], metadata !DIExpression(DW_OP_LLVM_fragment, 64, 64))
+; NOLOWER: call void @llvm.dbg.value(metadata i64 undef, {{.*}})
; NOLOWER-NOT: store
; NOLOWER: call void @tworegs_callee(i64 %o.coerce0, i64 %o.coerce1)
Added: llvm/trunk/test/Transforms/Mem2Reg/debug-alloca-phi-2.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Mem2Reg/debug-alloca-phi-2.ll?rev=334830&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/Mem2Reg/debug-alloca-phi-2.ll (added)
+++ llvm/trunk/test/Transforms/Mem2Reg/debug-alloca-phi-2.ll Fri Jun 15 06:48:55 2018
@@ -0,0 +1,46 @@
+; RUN: opt < %s -mem2reg -S | FileCheck %s
+source_filename = "bugpoint-output.bc"
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.12.0"
+
+define void @scan() #0 !dbg !12 {
+entry:
+ %entry1 = alloca i1, align 8
+ call void @llvm.dbg.declare(metadata i1* %entry1, metadata !18, metadata !19), !dbg !20
+ store i1 0, i1* %entry1, align 8, !dbg !20
+ br label %for.cond, !dbg !20
+
+for.cond:
+; CHECK: %[[PHI:.*]] = phi i1 [ false, %entry ], [ %0, %for.cond ]
+ %entryN = load i1, i1* %entry1, align 8, !dbg !20
+; CHECK: call void @llvm.dbg.value(metadata i1 %[[PHI]],
+; CHECK-SAME: metadata !DIExpression())
+ %0 = add i1 %entryN, 1
+; CHECK: %0 = add i1 %[[PHI]], true
+; CHECK: call void @llvm.dbg.value(metadata i1 %0,
+; CHECK-SAME: metadata !DIExpression())
+ store i1 %0, i1* %entry1, align 8, !dbg !20
+ br label %for.cond, !dbg !20
+}
+
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+
+attributes #0 = { nounwind ssp uwtable }
+attributes #1 = { nounwind readnone }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!10, !11}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "adrian", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug)
+!1 = !DIFile(filename: "<stdin>", directory: "/")
+!2 = !{}
+!4 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
+!10 = !{i32 2, !"Debug Info Version", i32 3}
+!11 = !{i32 1, !"PIC Level", i32 2}
+!12 = distinct !DISubprogram(name: "scan", scope: !1, file: !1, line: 4, type: !13, isLocal: false, isDefinition: true, scopeLine: 5, flags: DIFlagPrototyped, isOptimized: true, unit: !0, retainedNodes: !15)
+!13 = !DISubroutineType(types: !14)
+!14 = !{null, !4, !4}
+!15 = !{!18}
+!18 = !DILocalVariable(name: "entry", scope: !12, file: !1, line: 6, type: !4)
+!19 = !DIExpression()
+!20 = !DILocation(line: 6, scope: !12)
More information about the llvm-commits
mailing list