[PATCH] D51813: [Util] Refer to [s|z]exts of args when converting dbg.declares (fix PR35400)

[Vedant Kumar via Phabricator via llvm-commits]

vsk created this revision.
vsk added reviewers: dblaikie, aprantl.
vsk added a project: debug-info.
Herald added a subscriber: hiraditya.

When converting dbg.declares, if the described value is a [s|z]ext,
refer to the ext directly instead of referring to its operand.

This fixes a narrowing bug (the debugger got the sign of a variable
wrong, see llvm.org/PR35400).

The main reason to refer to the ext's operand was that an optimization
may remove the ext itself, leading to a dropped variable. Now that
InstCombine has been taught to use replaceAllDbgUsesWith (r336451), this
is less of a concern. Other passes can/should adopt this API as needed
to fix dropped variable bugs.


https://reviews.llvm.org/D51813

Files:
  llvm/lib/Transforms/Utils/Local.cpp
  llvm/test/Transforms/Util/dbg-user-of-aext.ll


Index: llvm/test/Transforms/Util/dbg-user-of-aext.ll
===================================================================
--- llvm/test/Transforms/Util/dbg-user-of-aext.ll
+++ llvm/test/Transforms/Util/dbg-user-of-aext.ll
@@ -1,6 +1,6 @@
 ; Checks that llvm.dbg.declare -> llvm.dbg.value conversion utility
-; (here exposed through the SROA) pass, properly inserts bit_piece expressions
-; if it only describes part of the variable.
+; (here exposed through the SROA) pass refers to [s|z]exts of values (as
+; opposed to the operand of a [s|z]ext).
 ; RUN: opt -S -sroa %s | FileCheck %s
 
 ; Built from:
@@ -22,8 +22,8 @@
 
 ; CHECK: call void @llvm.dbg.value(metadata i8 %g.coerce0, metadata ![[VAR_STRUCT:[0-9]+]], metadata !DIExpression(DW_OP_LLVM_fragment, 0, 8))
 ; CHECK: call void @llvm.dbg.value(metadata i64 %g.coerce1, metadata ![[VAR_STRUCT]], metadata !DIExpression(DW_OP_LLVM_fragment, 32, 64))
-; CHECK: call void @llvm.dbg.value(metadata i1 %b, metadata ![[VAR_BOOL:[0-9]+]], metadata !DIExpression())
-; CHECK: call void @llvm.dbg.value(metadata i1 %frag, metadata ![[VAR_FRAG:[0-9]+]], metadata !DIExpression(DW_OP_LLVM_fragment, 0, 1))
+; CHECK: call void @llvm.dbg.value(metadata i8 %frombool, metadata ![[VAR_BOOL:[0-9]+]], metadata !DIExpression())
+; CHECK: call void @llvm.dbg.value(metadata i8 %frombool1, metadata ![[VAR_FRAG:[0-9]+]], metadata !DIExpression(DW_OP_LLVM_fragment, 0, 4))
 
 %struct.foo = type { i8, i64 }
 
@@ -40,13 +40,16 @@
   %frombool = zext i1 %b to i8
   store i8 %frombool, i8* %b.addr, align 1
   call void @llvm.dbg.declare(metadata i8* %b.addr, metadata !15, metadata !16), !dbg !17
-  %frombool1 = zext i1 %frag to i8
+  %frombool1 = sext i1 %frag to i8
   store i8 %frombool1, i8* %frag.addr, align 1
   call void @llvm.dbg.declare(metadata i8* %frag.addr, metadata !18, metadata !23), !dbg !19
   call void @llvm.dbg.declare(metadata %struct.foo* %g, metadata !20, metadata !16), !dbg !21
   ret void, !dbg !22
 }
 
+; CHECK: ![[VAR_STRUCT]] = !DILocalVariable(name: "g"
+; CHECK: ![[VAR_BOOL]] = !DILocalVariable(name: "b"
+; CHECK: ![[VAR_FRAG]] = !DILocalVariable(name: "frag"
 
 declare void @llvm.dbg.declare(metadata, metadata, metadata)
 
Index: llvm/lib/Transforms/Utils/Local.cpp
===================================================================
--- llvm/lib/Transforms/Utils/Local.cpp
+++ llvm/lib/Transforms/Utils/Local.cpp
@@ -1288,33 +1288,6 @@
     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;
-  if (ZExtInst *ZExt = dyn_cast<ZExtInst>(SI->getOperand(0)))
-    ExtendedArg = dyn_cast<Argument>(ZExt->getOperand(0));
-  if (SExtInst *SExt = dyn_cast<SExtInst>(SI->getOperand(0)))
-    ExtendedArg = dyn_cast<Argument>(SExt->getOperand(0));
-  if (ExtendedArg) {
-    // If this DII was already describing only a fragment of a variable, ensure
-    // that fragment is appropriately narrowed here.
-    // But if a fragment wasn't used, describe the value as the original
-    // argument (rather than the zext or sext) so that it remains described even
-    // if the sext/zext is optimized away. This widens the variable description,
-    // leaving it up to the consumer to know how the smaller value may be
-    // represented in a larger register.
-    if (auto Fragment = DIExpr->getFragmentInfo()) {
-      unsigned FragmentOffset = Fragment->OffsetInBits;
-      SmallVector<uint64_t, 3> Ops(DIExpr->elements_begin(),
-                                   DIExpr->elements_end() - 3);
-      Ops.push_back(dwarf::DW_OP_LLVM_fragment);
-      Ops.push_back(FragmentOffset);
-      const DataLayout &DL = DII->getModule()->getDataLayout();
-      Ops.push_back(DL.getTypeSizeInBits(ExtendedArg->getType()));
-      DIExpr = Builder.createExpression(Ops);
-    }
-    DV = ExtendedArg;
-  }
   if (!LdStHasDebugValue(DIVar, DIExpr, SI))
     Builder.insertDbgValueIntrinsic(DV, DIVar, DIExpr, DII->getDebugLoc(),
                                     SI);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D51813.164508.patch
Type: text/x-patch
Size: 4064 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180907/f05ec312/attachment.bin>