Fix debug info for the return value of resource vector loads.

include/dxc/DXIL/DxilUtil.h

@@ -97,6 +97,7 @@ namespace dxilutil {
                                       unsigned startOpIdx,
                                       unsigned numOperands);
   bool SimplifyTrivialPHIs(llvm::BasicBlock *BB);
+  void ScatterDebugValueToVectorElements(llvm::Value *Val);
   std::unique_ptr<llvm::Module> LoadModuleFromBitcode(llvm::StringRef BC,
     llvm::LLVMContext &Ctx, std::string &DiagStr);
   std::unique_ptr<llvm::Module> LoadModuleFromBitcode(llvm::MemoryBuffer *MB,

lib/DXIL/DxilUtil.cpp

@@ -10,23 +10,25 @@
 ///////////////////////////////////////////////////////////////////////////////
 
 
-#include "llvm/IR/GlobalVariable.h"
 #include "dxc/DXIL/DxilTypeSystem.h"
 #include "dxc/DXIL/DxilUtil.h"
 #include "dxc/DXIL/DxilModule.h"
+#include "dxc/Support/Global.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/Twine.h"
 #include "llvm/Bitcode/ReaderWriter.h"
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/DiagnosticPrinter.h"
+#include "llvm/IR/GlobalVariable.h"
+#include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Constants.h"
+#include "llvm/IR/DIBuilder.h"
 #include "llvm/IR/IRBuilder.h"
-#include "dxc/Support/Global.h"
-#include "llvm/ADT/StringExtras.h"
-#include "llvm/ADT/Twine.h"
 
 using namespace llvm;
 using namespace hlsl;
@@ -326,6 +328,57 @@ bool SimplifyTrivialPHIs(BasicBlock *BB) {
   return Changed;
 }
 
+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;
+}
+
+// Propagates any llvm.dbg.value instruction for a given vector
+// to the elements that were used to create it through a series
+// of insertelement instructions.
+//
+// This is used after lowering a vector-returning intrinsic.
+// If we just keep the debug info on the recomposed vector,
+// we will lose it when we break it apart again during later
+// optimization stages.
+void ScatterDebugValueToVectorElements(Value *Val) {
+  DXASSERT(isa<InsertElementInst>(Val), "Should be a newly gathered vector.");
+  DbgValueInst *VecDbgValInst = FindDbgValueInst(Val);
+  if (VecDbgValInst == nullptr) return;
+
+  Type *ElemTy = Val->getType()->getVectorElementType();
+  DIBuilder DbgInfoBuilder(*VecDbgValInst->getModule());
+  unsigned ElemSizeInBits = VecDbgValInst->getModule()->getDataLayout().getTypeSizeInBits(ElemTy);
+
+  DIExpression *ParentBitPiece = VecDbgValInst->getExpression();
+  if (ParentBitPiece != nullptr && !ParentBitPiece->isBitPiece())
+    ParentBitPiece = nullptr;
+
+  while (InsertElementInst *InsertElt = dyn_cast<InsertElementInst>(Val)) {
+    Value *NewElt = InsertElt->getOperand(1);
+    unsigned EltIdx = static_cast<unsigned>(cast<ConstantInt>(InsertElt->getOperand(2))->getLimitedValue());
+    unsigned OffsetInBits = EltIdx * ElemSizeInBits;
+
+    if (ParentBitPiece) {
+      assert(OffsetInBits + ElemSizeInBits <= ParentBitPiece->getBitPieceSize()
+        && "Nested bit piece expression exceeds bounds of its parent.");
+      OffsetInBits += ParentBitPiece->getBitPieceOffset();
+    }
+
+    DIExpression *DIExpr = DbgInfoBuilder.createBitPieceExpression(OffsetInBits, ElemSizeInBits);
+    DbgInfoBuilder.insertDbgValueIntrinsic(NewElt, EltIdx, VecDbgValInst->getVariable(),
+      DIExpr, VecDbgValInst->getDebugLoc(), InsertElt);
+    Val = InsertElt->getOperand(0);
+  }
+}
+
 Value *SelectOnOperation(llvm::Instruction *Inst, unsigned operandIdx) {
   Instruction *prototype = Inst;
   for (unsigned i = 0; i < prototype->getNumOperands(); i++) {

lib/HLSL/HLOperationLower.cpp

@@ -2716,6 +2716,10 @@ void GenerateDxilSample(CallInst *CI, Function *F, ArrayRef<Value *> sampleArgs,
   // Replace ret val.
   CI->replaceAllUsesWith(retVal);
 
+  // Update the debug info
+  if (retVal->getType()->isVectorTy())
+    dxilutil::ScatterDebugValueToVectorElements(retVal);
+
   // get status
   if (status) {
     UpdateStatus(call, status, Builder, hlslOp);
@@ -3001,14 +3005,12 @@ void GenerateDxilGather(CallInst *CI, Function *F,
 
   CallInst *call = Builder.CreateCall(F, gatherArgs);
 
+  Value *retVal;
   if (!helper.hasSampleOffsets) {
     // extract value part
-    Value *retVal = ScalarizeResRet(CI->getType(), call, Builder);
-
-    // Replace ret val.
-    CI->replaceAllUsesWith(retVal);
+    retVal = ScalarizeResRet(CI->getType(), call, Builder);
   } else {
-    Value *retVal = UndefValue::get(CI->getType());
+    retVal = UndefValue::get(CI->getType());
     Value *elt = Builder.CreateExtractValue(call, (uint64_t)0);
     retVal = Builder.CreateInsertElement(retVal, elt, (uint64_t)0);
 
@@ -3026,10 +3028,16 @@ void GenerateDxilGather(CallInst *CI, Function *F,
     CallInst *callW = Builder.CreateCall(F, gatherArgs);
     elt = Builder.CreateExtractValue(callW, (uint64_t)3);
     retVal = Builder.CreateInsertElement(retVal, elt, 3);
-    // Replace ret val.
-    CI->replaceAllUsesWith(retVal);
+
     // TODO: UpdateStatus for each gather call.
   }
+
+  // Replace ret val.
+  CI->replaceAllUsesWith(retVal);
+
+  if (retVal->getType()->isVectorTy())
+    dxilutil::ScatterDebugValueToVectorElements(retVal);
+
   // Get status
   if (helper.status) {
     UpdateStatus(call, helper.status, Builder, hlslOp);
@@ -3327,6 +3335,7 @@ void TranslateLoad(ResLoadHelper &helper, HLResource::Kind RK,
     Value *retValNew = ScalarizeElements(Ty, ResultElts, Builder);
     helper.retVal->replaceAllUsesWith(retValNew);
     helper.retVal = retValNew;
+    if (Ty->isVectorTy()) dxilutil::ScatterDebugValueToVectorElements(retValNew);
     return;
   }
 
@@ -3433,6 +3442,9 @@ void TranslateLoad(ResLoadHelper &helper, HLResource::Kind RK,
   helper.retVal->replaceAllUsesWith(retValNew);
   // Save new ret val.
   helper.retVal = retValNew;
+  // Update debug info
+  if (retValNew->getType()->isVectorTy())
+    dxilutil::ScatterDebugValueToVectorElements(retValNew);
   // get status
   UpdateStatus(ResRet, helper.status, Builder, OP);
 }

lib/HLSL/HLSignatureLower.cpp

@@ -25,7 +25,6 @@
 
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/DebugInfo.h"
-#include "llvm/IR/DIBuilder.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Transforms/Utils/Local.h"
@@ -609,29 +608,6 @@ 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) {
@@ -643,38 +619,18 @@ void replaceDirectInputParameter(Value *param, Function *loadInput,
     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);
+
+    // THe individual loadInputs are the authoritative source of values for the vector.
+    dxilutil::ScatterDebugValueToVectorElements(newVec);
   } else if (!Ty->isArrayTy() && !HLMatrixType::isa(Ty)) {
     DXASSERT(cols == 1, "only support scalar here");
     Value *colIdx = hlslOP->GetU8Const(0);

tools/clang/test/CodeGenHLSL/debug/_readme.txt

@@ -0,0 +1,23 @@
+There is a confirmation bias problem when testing debug info using file-check.
+Say your test file contains:
+
+  // RUN: %dxc -E main -T vs_6_0 -Zi %s | FileCheck %s
+  // CHECK: foo
+  void main() {}
+
+Due to /Zi, the !dx.source.contents metadata will be present and contain a string
+with the original source file. This means that the generated file contains your
+"// CHECK: foo", and hence the "foo" itself, so the check will succeed by default!
+
+The current workaround is to include the following in your test to explicitly match
+the quoted source file:
+
+  // Exclude quoted source file (see readme)
+  // CHECK: {{!"[^"]*\\0A[^"]*"}}
+
+This will match a metadata string containing \0A (newline), which should only appear
+in the quoted source file. It will not match itself in the quoted source file because
+the regex won't match itself, and even less the escaped version of itself.
+
+Note that if you see a failure on that line, it means that something else before that
+CHECK failed to match.

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

@@ -11,8 +11,12 @@
 // statements below, since the source code for this test will be preserved in a
 // metadata element.
 
-// CHECK: !{{[0-9]+}} = !DIFile(filename: "{{[^:]*}}:{{[^:]*}}"
-// CHECK-NOT: !{{[0-9]+}} = !DIFile(
+// CHECK-NOT: !DIFile(
+// CHECK: !DIFile(filename: "{{[^:]*}}:{{[^:]*}}"
+// CHECK-NOT: !DIFile(
+
+// Exclude quoted source file (see readme)
+// CHECK: {{!"[^"]*\\0A[^"]*"}}
 
 Texture2D tex; // This is necessary for the second, non-bogus !DIFile
 void main() {}

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

@@ -1,18 +0,0 @@
-// 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

@@ -1,16 +0,0 @@
-// 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; }

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

@@ -0,0 +1,21 @@
+// 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: call i32 @dx.op.loadInput.i32
+// CHECK: call void @llvm.dbg.value
+// CHECK: call i32 @dx.op.loadInput.i32
+// CHECK: call void @llvm.dbg.value
+// CHECK: call void @dx.op.storeOutput.i32
+// CHECK: call void @dx.op.storeOutput.i32
+// CHECK: ret void
+
+// Exclude quoted source file (see readme)
+// CHECK: {{!"[^"]*\\0A[^"]*"}}
+
+// CHECK: !DILocalVariable(tag: DW_TAG_arg_variable, name: "a", arg: 1
+// CHECK: !DIExpression(DW_OP_bit_piece, 0, 32)
+// CHECK: !DIExpression(DW_OP_bit_piece, 32, 32)
+
+int2 main(int2 a : IN) : OUT { return a; }

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

@@ -0,0 +1,19 @@
+// 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: call i32 @dx.op.loadInput.i32
+// CHECK: call void @llvm.dbg.value
+// CHECK: call void @dx.op.storeOutput.i32
+// CHECK: ret void
+
+// Exclude quoted source file (see readme)
+// CHECK: {{!"[^"]*\\0A[^"]*"}}
+
+// CHECK: !DILocalVariable(tag: DW_TAG_arg_variable, name: "s", arg: 1
+// CHECK: !DIExpression(DW_OP_bit_piece, 96, 32)
+
+struct S { int2 a, b; };
+int main(S s : IN) : OUT { return s.b.y; }

tools/clang/test/CodeGenHLSL/debug/locals/structuredbuffer_load.hlsl

@@ -0,0 +1,25 @@
+// RUN: %dxc -E main -T vs_6_0 -Zi %s | FileCheck %s
+
+// Test that the debug information for the result of a texture load
+// is preserved after scalarization and optims.
+
+// CHECK: call %dx.types.ResRet.i32 @dx.op.bufferLoad.i32
+// CHECK: extractvalue %dx.types.ResRet.i32
+// CHECK: extractvalue %dx.types.ResRet.i32
+// CHECK: call void @llvm.dbg.value
+// CHECK: call void @llvm.dbg.value
+// CHECK: call void @dx.op.storeOutput.i32
+// CHECK: call void @dx.op.storeOutput.i32
+
+// Exclude quoted source file (see readme)
+// CHECK: {{!"[^"]*\\0A[^"]*"}}
+
+// CHECK: !DIExpression(DW_OP_bit_piece, 0, 32)
+// CHECK: !DIExpression(DW_OP_bit_piece, 32, 32)
+
+StructuredBuffer<int2> buf;
+int2 main() : OUT
+{
+    int2 result = buf.Load(0);
+    return result;
+}

tools/clang/test/CodeGenHLSL/debug/locals/texture_load.hlsl

@@ -0,0 +1,33 @@
+// RUN: %dxc -E main -T ps_6_0 -Zi %s | FileCheck %s
+
+// Test that the debug information for the result of a texture load
+// is preserved after scalarization and optims.
+
+// CHECK: call %dx.types.ResRet.f32 @dx.op.textureLoad.f32
+// CHECK: extractvalue %dx.types.ResRet.f32
+// CHECK: call void @llvm.dbg.value
+// CHECK: extractvalue %dx.types.ResRet.f32
+// CHECK: call void @llvm.dbg.value
+// CHECK: extractvalue %dx.types.ResRet.f32
+// CHECK: call void @llvm.dbg.value
+// CHECK: extractvalue %dx.types.ResRet.f32
+// CHECK: call void @llvm.dbg.value
+// CHECK: call void @dx.op.storeOutput.f32
+// CHECK: call void @dx.op.storeOutput.f32
+// CHECK: call void @dx.op.storeOutput.f32
+// CHECK: call void @dx.op.storeOutput.f32
+
+// Exclude quoted source file (see readme)
+// CHECK: {{!"[^"]*\\0A[^"]*"}}
+
+// CHECK: !DIExpression(DW_OP_bit_piece, 0, 32)
+// CHECK: !DIExpression(DW_OP_bit_piece, 32, 32)
+// CHECK: !DIExpression(DW_OP_bit_piece, 64, 32)
+// CHECK: !DIExpression(DW_OP_bit_piece, 96, 32)
+
+Texture1D<float4> tex;
+float4 main() : SV_Target
+{
+    float4 texel = tex.Load(0);
+    return texel;
+}

tools/clang/test/CodeGenHLSL/debug/locals/texture_sample.hlsl

@@ -0,0 +1,34 @@
+// RUN: %dxc -E main -T ps_6_0 -Zi %s | FileCheck %s
+
+// Test that the debug information for the result of a texture sample
+// is preserved after scalarization and optims.
+
+// CHECK: call %dx.types.ResRet.f32 @dx.op.sample.f32
+// CHECK: extractvalue %dx.types.ResRet.f32
+// CHECK: call void @llvm.dbg.value
+// CHECK: extractvalue %dx.types.ResRet.f32
+// CHECK: call void @llvm.dbg.value
+// CHECK: extractvalue %dx.types.ResRet.f32
+// CHECK: call void @llvm.dbg.value
+// CHECK: extractvalue %dx.types.ResRet.f32
+// CHECK: call void @llvm.dbg.value
+// CHECK: call void @dx.op.storeOutput.f32
+// CHECK: call void @dx.op.storeOutput.f32
+// CHECK: call void @dx.op.storeOutput.f32
+// CHECK: call void @dx.op.storeOutput.f32
+
+// Exclude quoted source file (see readme)
+// CHECK: {{!"[^"]*\\0A[^"]*"}}
+
+// CHECK: !DIExpression(DW_OP_bit_piece, 0, 32)
+// CHECK: !DIExpression(DW_OP_bit_piece, 32, 32)
+// CHECK: !DIExpression(DW_OP_bit_piece, 64, 32)
+// CHECK: !DIExpression(DW_OP_bit_piece, 96, 32)
+
+sampler samp;
+Texture2D<float4> tex;
+float4 main() : SV_Target
+{
+    float4 texel = tex.Sample(samp, 0);
+    return texel;
+}