Fix input vectors not having debug info (#2038)

We were systematically losing input vector debug info because of HLSignatureLower, which would replace them by per-element loadInput calls and then re-creating the vector through an insertelement chain. The llvm.dbg.value intrinsic was preserved for the recomposed vector, but this is not the authoritative source of data anymore - its elements are. Further optimizations would read past the insertelements and eventually we'd drop the vector and its debug info. To fix this, I'm now migrating the debug info to the individual vector elements as loaded from loadInput.

Also standardized on bitpieceexpression arguments being in bits everywhere, because it was becoming too confusing. The header for createBitPieceExpression had OffsetInBits while the source file had OffsetInBytes and various places converted counter-intuitively between the two.

lib/HLSL/HLSignatureLower.cpp

@@ -25,8 +25,10 @@
 
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/DebugInfo.h"
-#include "llvm/Transforms/Utils/Local.h"
+#include "llvm/IR/DIBuilder.h"
 #include "llvm/IR/IntrinsicInst.h"
+#include "llvm/IR/Module.h"
+#include "llvm/Transforms/Utils/Local.h"
 
 using namespace llvm;
 using namespace hlsl;
@@ -607,6 +609,29 @@ Value *replaceLdWithLdInput(Function *loadInput, LoadInst *ldInst,
   }
 }
 
+static DbgValueInst *findDbgValueInst(Value *Val) {
+  if (auto *ValAsMD = LocalAsMetadata::getIfExists(Val)) {
+    if (auto *ValMDAsVal = MetadataAsValue::getIfExists(Val->getContext(), ValAsMD)) {
+      for (User *ValMDUser : ValMDAsVal->users()) {
+        if (DbgValueInst *DbgValInst = dyn_cast<DbgValueInst>(ValMDUser))
+          return DbgValInst;
+      }
+    }
+  }
+  return nullptr;
+}
+
+static DIExpression *createNestedBitPieceExpression(DIBuilder &DbgInfoBuilder,
+  DIExpression* ParentDIExpr, unsigned OffsetInBits, unsigned SizeInBits) {
+  if (ParentDIExpr && ParentDIExpr->isBitPiece()) {
+    assert(OffsetInBits + SizeInBits <= ParentDIExpr->getBitPieceSize()
+      && "Nested bit piece expression exceeds bounds of its parent.");
+    OffsetInBits += ParentDIExpr->getBitPieceOffset();
+  }
+
+  return DbgInfoBuilder.createBitPieceExpression(OffsetInBits, SizeInBits);
+}
+
 void replaceDirectInputParameter(Value *param, Function *loadInput,
                                  unsigned cols, MutableArrayRef<Value *> args,
                                  bool bCast, OP *hlslOP, IRBuilder<> &Builder) {
@@ -617,13 +642,38 @@ void replaceDirectInputParameter(Value *param, Function *loadInput,
   if (VectorType *VT = dyn_cast<VectorType>(Ty)) {
     Value *newVec = llvm::UndefValue::get(VT);
     DXASSERT(cols == VT->getNumElements(), "vec size must match");
+
+    // We're about to essentially SROA a vector,
+    // since we'll load elements individually and reconstruct it.
+    // We need to migrate the debug information from the vector
+    // to the constituing scalars since those are the original
+    // source of the data. Optimizations will later make the vector disappear,
+    // and we'll lose debug information applied to it.
+    DbgValueInst *DbgValInst = findDbgValueInst(param);
+    DIBuilder DbgInfoBuilder(*loadInput->getParent());
+    unsigned ElemSizeInBits = loadInput->getParent()->getDataLayout()
+      .getTypeAllocSize(VT->getElementType()) * 8;
+
     for (unsigned col = 0; col < cols; col++) {
       Value *colIdx = hlslOP->GetU8Const(col);
       args[DXIL::OperandIndex::kLoadInputColOpIdx] = colIdx;
       Value *input =
           GenerateLdInput(loadInput, args, Builder, zero, bCast, EltTy);
       newVec = Builder.CreateInsertElement(newVec, input, col);
+      if (DbgValInst) {
+        unsigned OffsetInBits = col * ElemSizeInBits;
+        DIExpression *DIExpr = createNestedBitPieceExpression(DbgInfoBuilder,
+          DbgValInst->getExpression(), OffsetInBits, ElemSizeInBits);
+        DbgInfoBuilder.insertDbgValueIntrinsic(input, 0, DbgValInst->getVariable(), DIExpr,
+          DbgValInst->getDebugLoc(), cast<Instruction>(newVec));
+      }
     }
+
+    // This will relocate the DbgValueInst to the newly created vector.
+    // We will eventually lose it as optimizations realize that the
+    // new authoritative source of data are the loadInput instructions
+    // that we generated above, but this is fine since we created
+    // DbgValueInsts for them too.
     param->replaceAllUsesWith(newVec);
   } else if (!Ty->isArrayTy() && !HLMatrixType::isa(Ty)) {
     DXASSERT(cols == 1, "only support scalar here");
@@ -631,7 +681,7 @@ void replaceDirectInputParameter(Value *param, Function *loadInput,
     args[DXIL::OperandIndex::kLoadInputColOpIdx] = colIdx;
     Value *input =
         GenerateLdInput(loadInput, args, Builder, zero, bCast, EltTy);
-    param->replaceAllUsesWith(input);
+    param->replaceAllUsesWith(input); // Will properly relocate any DbgValueInst
   } else if (HLMatrixType::isa(Ty)) {
     if (param->use_empty()) return;
     DXASSERT(param->hasOneUse(),

lib/IR/DIBuilder.cpp

@@ -623,13 +623,11 @@ DIExpression *DIBuilder::createExpression(ArrayRef<int64_t> Signed) {
   return createExpression(Addr);
 }
 
-DIExpression *DIBuilder::createBitPieceExpression(unsigned OffsetInBytes,
-                                                  unsigned SizeInBytes) {
-#if 0 // HLSL Change
-  uint64_t Addr[] = {dwarf::DW_OP_bit_piece, OffsetInBytes, SizeInBytes};
-#else // HLSL Change
-  uint64_t Addr[] = {dwarf::DW_OP_bit_piece, OffsetInBytes*8, SizeInBytes*8};
-#endif // HLSL Change
+// HLSL Begin Change: Match -InBits suffixes from header
+DIExpression *DIBuilder::createBitPieceExpression(unsigned OffsetInBits, 
+                                                  unsigned SizeInBits) {
+  uint64_t Addr[] = {dwarf::DW_OP_bit_piece, OffsetInBits, SizeInBits};
+// HLSL End Change
   return DIExpression::get(VMContext, Addr);
 }
 

lib/Transforms/Scalar/SROA.cpp

@@ -4327,11 +4327,7 @@ bool SROA::splitAlloca(AllocaInst &AI, AllocaSlices &AS) {
             continue;
           Size = std::min(Size, AbsEnd - Start);
         }
-#if 0 // HLSL Change
         PieceExpr = DIB.createBitPieceExpression(Start, Size);
-#else // HLSL Change
-        PieceExpr = DIB.createBitPieceExpression(Start / 8, Size / 8);
-#endif // HLSL Change
       }
 
       // Remove any existing dbg.declare intrinsic describing the same alloca.

lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp

@@ -1685,11 +1685,10 @@ bool SROA_HLSL::performScalarRepl(Function &F, DxilTypeSystem &typeSys) {
 
             DIExpression *DDIExp = nullptr;
             if (parentOffset+debugOffset == 0 && DL.getTypeAllocSize(AI->getAllocatedType()) == size) {
-              std::vector<uint64_t> args;
-              DDIExp = DIB.createExpression(args);
+              DDIExp = DIB.createExpression();
             }
             else {
-              DDIExp = DIB.createBitPieceExpression(parentOffset+debugOffset, size);
+              DDIExp = DIB.createBitPieceExpression((parentOffset+debugOffset) * 8, size * 8);
             }
             OffsetMap[Elt] = parentOffset+debugOffset;
 #endif // HLSL Change
@@ -5692,11 +5691,10 @@ void SROA_Parameter_HLSL::flattenArgument(
           argTy = argTy->getPointerElementType();
         DIExpression *DDIExp = nullptr;
         if (debugOffset == 0 && DL.getTypeAllocSize(argTy) == size) {
-          std::vector<uint64_t> Addr;
-          DDIExp = DIB.createExpression(Addr);
+          DDIExp = DIB.createExpression();
         }
         else {
-          DDIExp = DIB.createBitPieceExpression(debugOffset, size);
+          DDIExp = DIB.createBitPieceExpression(debugOffset * 8, size * 8);
         }
 #endif // HLSL Change
         debugOffset += size;

lib/Transforms/Scalar/Scalarizer.cpp

@@ -693,21 +693,21 @@ bool Scalarizer::finish() {
           Type *Ty = Op->getType();
           unsigned Count = Ty->getVectorNumElements();
           Type *EltTy = Ty->getVectorElementType();
-          unsigned EltSize = DL.getTypeSizeInBits(EltTy);
+          unsigned EltSizeInBits = DL.getTypeSizeInBits(EltTy);
           for (User *U : DINode->users())
             if (DbgValueInst *DVI = dyn_cast<DbgValueInst>(U)) {
               DIBuilder DIB(M, /*AllowUnresolved*/ false);
               auto *VarInfo = DVI->getVariable();
               DebugLoc DbgLoc = DVI->getDebugLoc();
-              unsigned Offset = 0;
+              unsigned OffsetInBits = 0;
               for (unsigned I = 0; I < Count; ++I) {
                 // TODO: need to use DIExpression::createFragmentExpression for
                 // case DVI->getExpression is already bit piece.
                 DIExpression *EltExpr =
-                    DIB.createBitPieceExpression(Offset / 8, EltSize / 8);
-                Offset += EltSize;
+                    DIB.createBitPieceExpression(OffsetInBits, EltSizeInBits);
+                OffsetInBits += EltSizeInBits;
 
-                DIB.insertDbgValueIntrinsic(CV[I], Offset, VarInfo, EltExpr,
+                DIB.insertDbgValueIntrinsic(CV[I], OffsetInBits, VarInfo, EltExpr,
                                             DbgLoc, DVI);
               }
             }

tools/clang/test/CodeGenHLSL/debug/input_vector.hlsl

@@ -0,0 +1,18 @@
+// RUN: %dxc -E main -T vs_6_0 -Zi %s | FileCheck %s
+
+// Test that an input vector's debug information is preserved,
+// especially through its lowering to per-element loadInputs.
+
+// CHECK: %[[xval:.*]] = call i32 @dx.op.loadInput.i32(i32 4, i32 0, i32 0, i8 0, i32 undef)
+// CHECK: call void @llvm.dbg.value(metadata i32 %[[xval]], i64 0, metadata ![[var:.*]], metadata ![[xexpr:.*]])
+// CHECK: %[[yval:.*]] = call i32 @dx.op.loadInput.i32(i32 4, i32 0, i32 0, i8 1, i32 undef)
+// CHECK: call void @llvm.dbg.value(metadata i32 %[[yval]], i64 0, metadata ![[var]], metadata ![[yexpr:.*]])
+// CHECK: call void @dx.op.storeOutput.i32(i32 5, i32 0, i32 0, i8 0, i32 %[[xval]])
+// CHECK: call void @dx.op.storeOutput.i32(i32 5, i32 0, i32 0, i8 1, i32 %[[yval]])
+// CHECK: ret void
+
+// CHECK: ![[var]] = !DILocalVariable(tag: DW_TAG_arg_variable, name: "a", arg: 1
+// CHECK: ![[xexpr]] = !DIExpression(DW_OP_bit_piece, 0, 32)
+// CHECK: ![[yexpr]] = !DIExpression(DW_OP_bit_piece, 32, 32)
+
+int2 main(int2 a : IN) : OUT { return a; }

tools/clang/test/CodeGenHLSL/debug/input_vector_in_struct.hlsl

@@ -0,0 +1,16 @@
+// RUN: %dxc -E main -T vs_6_0 -Zi %s | FileCheck %s
+
+// Test that debug offsets are correct after lowering a vector
+// input to loadInput calls, when the vector input itself was
+// offset in a struct.
+
+// CHECK: %[[val:.*]] = call i32 @dx.op.loadInput.i32(i32 4, i32 1, i32 0, i8 1, i32 undef)
+// CHECK: call void @llvm.dbg.value(metadata i32 %[[val]], i64 0, metadata ![[var:.*]], metadata ![[expr:.*]])
+// CHECK: call void @dx.op.storeOutput.i32(i32 5, i32 0, i32 0, i8 0, i32 %[[val]])
+// CHECK: ret void
+
+// CHECK: ![[var]] = !DILocalVariable(tag: DW_TAG_arg_variable, name: "s", arg: 1
+// CHECK: ![[expr]] = !DIExpression(DW_OP_bit_piece, 96, 32)
+
+struct S { int2 a, b; };
+int main(S s : IN) : OUT { return s.b.y; }