Addressed review feedback

converted DW_OP_LLVM_fragment expr to a DW_OP_LLVM_convert expr

Author: rastogishubham

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

@@ -32,6 +32,7 @@
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/Analysis/VectorUtils.h"
+#include "llvm/BinaryFormat/Dwarf.h"
 #include "llvm/CodeGen/ByteProvider.h"
 #include "llvm/CodeGen/DAGCombine.h"
 #include "llvm/CodeGen/ISDOpcodes.h"
@@ -14457,29 +14458,35 @@ static SDValue tryToFoldExtOfLoad(SelectionDAG &DAG, DAGCombiner &Combiner,
                                    LN0->getBasePtr(), N0.getValueType(),
                                    LN0->getMemOperand());
   Combiner.ExtendSetCCUses(SetCCs, N0, ExtLoad, ExtOpc);
+  unsigned Opcode = N->getOpcode();
+  bool IsSigned = Opcode == ISD::SIGN_EXTEND ? true : false;
   // If the load value is used only by N, replace it via CombineTo N.
   SDValue OldLoadVal(LN0, 0);
-  SDValue OldSextValue(N, 0);
+  SDValue OldExtValue(N, 0);
   bool NoReplaceTrunc = OldLoadVal.hasOneUse();
   Combiner.CombineTo(N, ExtLoad);
 
   // Because we are replacing a load and a sext with a load-sext instruction,
   // the dbg_value attached to the load will be of a smaller bit width, and we
   // have to add a DW_OP_LLVM_fragment to the DIExpression.
-  auto SalvageToOldLoadSize = [&](SDValue From, SDValue To64) {
+  auto SalvageToOldLoadSize = [&](SDValue From, SDValue To, bool IsSigned) {
     for (SDDbgValue *Dbg : DAG.GetDbgValues(From.getNode())) {
-      unsigned VarBits = From->getValueSizeInBits(0);
+      unsigned VarBitsFrom = From->getValueSizeInBits(0);
+      unsigned VarBitsTo = To->getValueSizeInBits(0);
 
-      // Build/append a fragment expression [0, VarBits]
+      // Build a convert expression for the s|z extend.
       const DIExpression *OldE = Dbg->getExpression();
-      auto NewE = DIExpression::createFragmentExpression(OldE, 0, VarBits);
-
-      // Create a new SDDbgValue that points at the widened node with the
-      // fragment.
-      if (!NewE)
-        continue;
+      SmallVector<uint64_t, 8> Ops;
+      dwarf::TypeKind TK =
+          IsSigned ? dwarf::DW_ATE_signed : dwarf::DW_ATE_unsigned;
+      Ops.append({dwarf::DW_OP_LLVM_convert, VarBitsFrom, TK,
+                  dwarf::DW_OP_LLVM_convert, VarBitsTo, TK});
+      auto *NewE = DIExpression::get(OldE->getContext(), Ops);
+
+      // // Create a new SDDbgValue that points at the widened node with the
+      // // fragment.
       SDDbgValue *NewDV = DAG.getDbgValue(
-          Dbg->getVariable(), *NewE, To64.getNode(), To64.getResNo(),
+          Dbg->getVariable(), NewE, To.getNode(), To.getResNo(),
           Dbg->isIndirect(), Dbg->getDebugLoc(), Dbg->getOrder());
       DAG.AddDbgValue(NewDV, /*isParametet*/ false);
     }
@@ -14488,21 +14495,21 @@ static SDValue tryToFoldExtOfLoad(SelectionDAG &DAG, DAGCombiner &Combiner,
   if (NoReplaceTrunc) {
     if (LN0->getHasDebugValue()) {
       DAG.transferDbgValues(OldLoadVal, ExtLoad);
-      SalvageToOldLoadSize(OldLoadVal, ExtLoad);
+      SalvageToOldLoadSize(OldLoadVal, ExtLoad, IsSigned);
     }
     if (N->getHasDebugValue())
-      DAG.transferDbgValues(OldSextValue, ExtLoad);
+      DAG.transferDbgValues(OldExtValue, ExtLoad);
     DAG.ReplaceAllUsesOfValueWith(SDValue(LN0, 1), ExtLoad.getValue(1));
     Combiner.recursivelyDeleteUnusedNodes(LN0);
   } else {
     SDValue Trunc =
         DAG.getNode(ISD::TRUNCATE, SDLoc(N0), N0.getValueType(), ExtLoad);
     if (LN0->getHasDebugValue()) {
       DAG.transferDbgValues(OldLoadVal, Trunc);
-      SalvageToOldLoadSize(OldLoadVal, Trunc);
+      SalvageToOldLoadSize(OldLoadVal, Trunc, IsSigned);
     }
     if (N->getHasDebugValue())
-      DAG.transferDbgValues(OldSextValue, Trunc);
+      DAG.transferDbgValues(OldExtValue, Trunc);
     Combiner.CombineTo(LN0, Trunc, ExtLoad.getValue(1));
   }
   return SDValue(N, 0); // Return N so it doesn't get rechecked!

llvm/test/DebugInfo/X86/selectionDAG-load-sext.mir

@@ -6,12 +6,12 @@
 
 # MIR: ![[IDX:[0-9]+]] = !DILocalVariable(name: "Idx"
 # MIR-LABEL: bb.0
-# MIR: %{{[0-9a-zA-Z]+}}{{.*}} = MOVSX64rm32 ${{.*}}, 1, $noreg, @GlobArr, $noreg, debug-instr-number [[INSTR_NUM:[0-9]+]]
+# MIR: %{{[0-9a-f]+}}{{.*}} = MOVSX64rm32 ${{.*}}, 1, $noreg, @GlobArr, $noreg, debug-instr-number [[INSTR_NUM:[0-9]+]]
 # MIR-NEXT: DBG_INSTR_REF ![[IDX]], !DIExpression(DW_OP_LLVM_arg, 0), dbg-instr-ref([[INSTR_NUM]], 0)
-# MIR-NEXT: DBG_INSTR_REF ![[IDX]], !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_fragment, 0, 32), dbg-instr-ref([[INSTR_NUM]], 0)
+# MIR-NEXT: DBG_INSTR_REF ![[IDX]], !DIExpression(DW_OP_LLVM_arg, 0,  DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_LLVM_convert, 64, DW_ATE_signed), dbg-instr-ref([[INSTR_NUM]], 0)
 
-# DUMP: DW_AT_location	(indexed ({{[0-9]+}}x{{[0-9]+}}) loclist = 0x{{[0-9]+}}: 
-# DUMP-NEXT: [0x{{[0-9]+}}, 0x{{[0-9]+}}): {{.*}}, DW_OP_piece 0x4
+# DUMP: DW_AT_location	(indexed ({{[0-9a-f]+}}x{{[0-9a-f]+}}) loclist = 0x{{[0-9]+}}: 
+# DUMP-NEXT: [0x{{[0-9a-f]+}}, 0x{{[0-9a-f]+}}): DW_OP_breg0 RAX+0, DW_OP_convert (0x{{[0-9a-f]+}}) "DW_ATE_signed_32", DW_OP_convert (0x{{[0-9a-f]+}}) "DW_ATE_signed_64")
 
 --- |
   @GlobArr = dso_local local_unnamed_addr global [5 x i32] [i32 1, i32 1, i32 2, i32 3, i32 5], align 16, !dbg !0