[llvm] r334704 - [DebugInfo] Check size of variable in ConvertDebugDeclareToDebugValue

Bjorn Pettersson via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 14 04:23:42 PDT 2018


Author: bjope
Date: Thu Jun 14 04:23:42 2018
New Revision: 334704

URL: http://llvm.org/viewvc/llvm-project?rev=334704&view=rev
Log:
[DebugInfo] Check size of variable in ConvertDebugDeclareToDebugValue

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/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=334704&r1=334703&r2=334704&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/IntrinsicInst.h (original)
+++ llvm/trunk/include/llvm/IR/IntrinsicInst.h Thu Jun 14 04:23:42 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=334704&r1=334703&r2=334704&view=diff
==============================================================================
--- llvm/trunk/lib/IR/IntrinsicInst.cpp (original)
+++ llvm/trunk/lib/IR/IntrinsicInst.cpp Thu Jun 14 04:23:42 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=334704&r1=334703&r2=334704&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/Local.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/Local.cpp Thu Jun 14 04:23:42 2018
@@ -1228,6 +1228,23 @@ static bool PhiHasDebugValue(DILocalVari
   return false;
 }
 
+/// Check if the store 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
+/// store 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.getTypeStoreSizeInBits(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();
 

Modified: llvm/trunk/test/Transforms/InstCombine/debuginfo.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/debuginfo.ll?rev=334704&r1=334703&r2=334704&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/debuginfo.ll (original)
+++ llvm/trunk/test/Transforms/InstCombine/debuginfo.ll Thu Jun 14 04:23:42 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=334704&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/Mem2Reg/debug-alloca-phi-2.ll (added)
+++ llvm/trunk/test/Transforms/Mem2Reg/debug-alloca-phi-2.ll Thu Jun 14 04:23:42 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