Recover lost debug info for returned variable. (#2869)

lib/Transforms/Scalar/DxilConditionalMem2Reg.cpp

@@ -14,10 +14,13 @@
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Dominators.h"
+#include "llvm/IR/Module.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/IR/LegacyPassManager.h"
+#include "llvm/IR/DebugInfo.h"
+#include "llvm/IR/DIBuilder.h"
 
 #include "dxc/DXIL/DxilUtil.h"
 #include "dxc/HLSL/HLModule.h"
@@ -241,14 +244,166 @@ public:
     return Changed;
   }
 
-  bool runOnFunction(Function &F) override {
+  struct StoreInfo {
+    Value *V;
+    unsigned Offset;
+  };
+  static bool FindAllStores(Module &M, Value *V, SmallVectorImpl<StoreInfo> *Stores) {
+    SmallVector<StoreInfo, 8> Worklist;
+    std::set<Value *> Seen;
+
+    auto Add = [&](Value *V, unsigned OffsetInBits) {
+      if (Seen.insert(V).second)
+        Worklist.push_back({ V, OffsetInBits });
+    };
+
+    Add(V, 0);
+
+    const DataLayout &DL = M.getDataLayout();
+
+    while (Worklist.size()) {
+      auto Info = Worklist.pop_back_val();
+      auto *Elem = Info.V;
+
+      if (auto GEP = dyn_cast<GEPOperator>(Elem)) {
+        if (GEP->getNumIndices() != 2)
+          continue;
+
+        unsigned ElemSize = 0;
+
+        Type *GEPPtrType = GEP->getPointerOperand()->getType();
+        Type *PtrElemType = GEPPtrType->getPointerElementType();
+        if (ArrayType *ArrayTy = dyn_cast<ArrayType>(PtrElemType)) {
+          ElemSize = DL.getTypeAllocSizeInBits(ArrayTy->getElementType());
+        }
+        else if (VectorType *VectorTy = dyn_cast<VectorType>(PtrElemType)) {
+          ElemSize = DL.getTypeAllocSizeInBits(VectorTy->getElementType());
+        }
+        else {
+          return false;
+        }
+
+        unsigned OffsetInBits = 0;
+        for (unsigned i = 0; i < GEP->getNumIndices(); i++) {
+          auto IdxOp = dyn_cast<ConstantInt>(GEP->getOperand(i+1));
+          if (!IdxOp) {
+            return false;
+          }
+          uint64_t Idx = IdxOp->getLimitedValue();
+          if (i == 0) {
+            if (Idx != 0)
+              return false;
+          }
+          else {
+            OffsetInBits = Idx * ElemSize;
+          }
+        }
+
+        for (User *U : Elem->users())
+          Add(U, Info.Offset + OffsetInBits);
+      }
+      else if (auto *Store = dyn_cast<StoreInst>(Elem)) {
+        Stores->push_back({ Store, Info.Offset });
+      }
+    }
+
+    return true;
+  }
 
+  // Function to rewrite debug info for output argument.
+  // Sometimes, normal local variables that get returned from functions get rewritten as
+  // a pointer argument.
+  //
+  // Right now, we generally have a single dbg.declare for the Argument, but as we lower
+  // it to storeOutput, the dbg.declare and the Argument both get removed, leavning no
+  // debug info for the local variable.
+  //
+  // Solution here is to rewrite the dbg.declare as dbg.value's by finding all the stores
+  // and writing a dbg.value immediately before the store. Fairly conservative at the moment 
+  // about what cases to rewrite (only scalars and vectors, and arrays of scalars and vectors).
+  //
+  bool RewriteOutputArgsDebugInfo(Function &F) {
+    bool Changed = false;
+    Module *M = F.getParent();
+    DIBuilder DIB(*M);
+
+    SmallVector<StoreInfo, 4> Stores;
+    LLVMContext &Ctx = F.getContext();
+    for (Argument &Arg : F.args()) {
+      if (!Arg.getType()->isPointerTy())
+        continue;
+      Type *Ty = Arg.getType()->getPointerElementType();
+
+      bool IsSimpleType =
+        Ty->isSingleValueType() ||
+        Ty->isVectorTy() ||
+        (Ty->isArrayTy() && (Ty->getArrayElementType()->isVectorTy() || Ty->getArrayElementType()->isSingleValueType()));
+
+      if (!IsSimpleType)
+        continue;
+
+      Stores.clear();
+      for (User *U : Arg.users()) {
+        if (!FindAllStores(*M, U, &Stores)) {
+          Stores.clear();
+          break;
+        }
+      }
 
+      if (Stores.empty())
+        continue;
+
+      DbgDeclareInst *Declare = nullptr;
+      if (auto *L = LocalAsMetadata::getIfExists(&Arg)) {
+        if (auto *DINode = MetadataAsValue::getIfExists(Ctx, L)) {
+          if (!DINode->user_empty() && std::next(DINode->user_begin()) == DINode->user_end()) {
+            Declare = dyn_cast<DbgDeclareInst>(*DINode->user_begin());
+          }
+        }
+      }
+
+      if (Declare) {
+        DITypeIdentifierMap EmptyMap;
+        DILocalVariable *Var = Declare->getVariable();
+        DIExpression *Expr = Declare->getExpression();
+        DIType *VarTy = Var->getType().resolve(EmptyMap);
+        uint64_t VarSize = VarTy->getSizeInBits();
+        uint64_t Offset = 0;
+        if (Expr->isBitPiece())
+          Offset = Expr->getBitPieceOffset();
+
+        for (auto &Info : Stores) {
+          auto *Store = cast<StoreInst>(Info.V);
+          auto Val = Store->getValueOperand();
+          auto Loc = Store->getDebugLoc();
+          auto &M = *F.getParent();
+          unsigned ValSize = M.getDataLayout().getTypeAllocSizeInBits(Val->getType());
+
+          DIExpression *NewExpr = nullptr;
+          if (Offset || VarSize > ValSize) {
+            uint64_t Elems[] = { dwarf::DW_OP_bit_piece, Offset + Info.Offset, ValSize };
+            NewExpr = DIExpression::get(Ctx, Elems);
+          }
+          else {
+            NewExpr = DIExpression::get(Ctx, {});
+          }
+          DIB.insertDbgValueIntrinsic(Val, 0, Var, NewExpr, Loc, Store);
+        }
+
+        Declare->eraseFromParent();
+        Changed = true;
+      }
+    }
+
+    return Changed;
+  }
+
+  bool runOnFunction(Function &F) override {
     DominatorTree *DT = &getAnalysis<DominatorTreeWrapperPass>().getDomTree();
     AssumptionCache *AC = &getAnalysis<AssumptionCacheTracker>().getAssumptionCache(F);
-
     bool Changed = false;
-    
+
+    Changed |= RewriteOutputArgsDebugInfo(F);
     Changed |= RemoveAllUnusedAllocas(F);
     Changed |= ScalarizePreciseVectorAlloca(F);
     Changed |= Mem2Reg(F, *DT, *AC);

lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp

@@ -5715,6 +5715,7 @@ void SROA_Parameter_HLSL::createFlattenedFunction(Function *F) {
         funcAnnotation->GetRetTypeAnnotation();
     Module &M = *m_pHLModule->GetModule();
     Type *voidTy = Type::getVoidTy(m_pHLModule->GetCtx());
+#if 0 // We don't really want this to show up in debug info.
     // Create DbgDecl for the ret value.
     if (DISubprogram *funcDI = getDISubprogram(F)) {
         DITypeRef RetDITyRef = funcDI->getType()->getTypeArray()[0];
@@ -5728,6 +5729,7 @@ void SROA_Parameter_HLSL::createFlattenedFunction(Function *F) {
         DILocation *DL = DILocation::get(F->getContext(), funcDI->getLine(), 0, funcDI);
         DIB.insertDeclare(retValAddr, RetVar, Expr, DL, Builder.GetInsertPoint());
     }
+#endif
     for (BasicBlock &BB : F->getBasicBlockList()) {
       if (ReturnInst *RI = dyn_cast<ReturnInst>(BB.getTerminator())) {
         // Create store for return.

tools/clang/test/HLSLFileCheck/dxil/debug/vs_output.hlsl

@@ -0,0 +1,67 @@
+// RUN: %dxc /T vs_6_0 /E BezierVS /DDX12 /Od /Zi %s | FileCheck %s
+
+// CHECK-DAG: call void @llvm.dbg.value(metadata float %{{.+}}, i64 0, metadata !{{[0-9]+}}, metadata !{{[0-9]+}}), !dbg !{{[0-9]+}} ; var:"output" !DIExpression(DW_OP_bit_piece, 0, 32)
+// CHECK-DAG: call void @llvm.dbg.value(metadata float %{{.+}}, i64 0, metadata !{{[0-9]+}}, metadata !{{[0-9]+}}), !dbg !{{[0-9]+}} ; var:"output" !DIExpression(DW_OP_bit_piece, 32, 32)
+// CHECK-DAG: call void @llvm.dbg.value(metadata float %{{.+}}, i64 0, metadata !{{[0-9]+}}, metadata !{{[0-9]+}}), !dbg !{{[0-9]+}} ; var:"output" !DIExpression(DW_OP_bit_piece, 64, 32)
+
+//--------------------------------------------------------------------------------------
+// SimpleBezier.hlsl
+//
+// Shader demonstrating DirectX 11 tesselation of a bezier surface
+//
+// Advanced Technology Group (ATG)
+// Copyright (C) Microsoft Corporation. All rights reserved.
+//--------------------------------------------------------------------------------------
+
+#ifdef DX12
+#define RS \
+[RootSignature(" \
+    RootFlags(ALLOW_INPUT_ASSEMBLER_INPUT_LAYOUT), \
+    DescriptorTable(CBV(b0, numDescriptors = 1), visibility=SHADER_VISIBILITY_ALL) \
+")]
+#else
+#define RS
+#endif
+
+//--------------------------------------------------------------------------------------
+// Constant Buffers
+//--------------------------------------------------------------------------------------
+cbuffer cbPerFrame : register( b0 )
+{
+    matrix g_mViewProjection;
+    float3 g_cameraWorldPos;
+    float  g_tessellationFactor;
+};
+
+//--------------------------------------------------------------------------------------
+// Vertex shader section
+//--------------------------------------------------------------------------------------
+struct VS_CONTROL_POINT_INPUT
+{
+    float3 pos      : POSITION;
+};
+
+struct VS_CONTROL_POINT_OUTPUT
+{
+    float3 pos      : POSITION;
+};
+
+//--------------------------------------------------------------------------------------
+// This simple vertex shader passes the control points straight through to the
+// hull shader.  In a more complex scene, you might transform the control points
+// or perform skinning at this step.
+
+// The input to the vertex shader comes from the vertex buffer.
+
+// The output from the vertex shader will go into the hull shader.
+//--------------------------------------------------------------------------------------
+RS
+VS_CONTROL_POINT_OUTPUT BezierVS( VS_CONTROL_POINT_INPUT Input )
+{
+    VS_CONTROL_POINT_OUTPUT output;
+
+    output.pos = Input.pos;
+
+    return output;
+}
+