PIX value-to-declare pass: subprogram type members (#3140)

lib/DxilDia/DxcPixLiveVariables_FragmentIterator.cpp

@@ -232,6 +232,7 @@ unsigned SizeIfBaseType(llvm::DIType const* diType)
 
 void CompositeTypeFragmentIterator::DetermineStructMemberSizesAndOffsets(llvm::DIType const*diType, uint64_t BaseOffset)
 {
+  assert(diType->getTag() != llvm::dwarf::DW_TAG_subroutine_type);
   if (diType->getTag() == llvm::dwarf::DW_TAG_member)
   {
     BaseOffset += diType->getOffsetInBits();
@@ -340,8 +341,10 @@ dxil_debug_info::CreateMemberIterator(llvm::DbgDeclareInst *DbgDeclare,
     } else {
       llvm::DICompositeType *CT = llvm::dyn_cast<llvm::DICompositeType>(
           DbgDeclare->getVariable()->getType());
-      if (CT != nullptr && Expression->getNumElements() == 0) {
-        Iter.reset(new CompositeTypeFragmentIterator(CT));
+      if (CT != nullptr && Expression->getNumElements() == 0 ) {
+          if (CT->getTag() != llvm::dwarf::DW_TAG_subroutine_type) {
+              Iter.reset(new CompositeTypeFragmentIterator(CT));
+          }
       } else {
         Iter.reset(
             new DILayoutFragmentIterator(DataLayout, Alloca, Expression));

lib/DxilDia/DxcPixTypes.h

@@ -218,7 +218,7 @@ private:
 #ifndef NDEBUG
     for (auto *Node : m_pStruct->getElements())
     {
-      assert(llvm::isa<llvm::DIDerivedType>(Node));
+      assert(llvm::isa<llvm::DIDerivedType>(Node) || llvm::isa<llvm::DISubprogram>(Node));
     }
 #endif  // !NDEBUG
   }

lib/DxilDia/DxilDiaDataSource.cpp

@@ -156,6 +156,14 @@ STDMETHODIMP dxil_dia::DataSource::openSession(_COM_Outptr_ IDiaSession **ppSess
   *ppSession = nullptr;
   if (m_module.get() == nullptr)
     return E_FAIL;
+
+  ::llvm::sys::fs::MSFileSystem *msfPtr;
+  IFT(CreateMSFileSystemForDisk(&msfPtr));
+  std::unique_ptr<::llvm::sys::fs::MSFileSystem> msf(msfPtr);
+
+  ::llvm::sys::fs::AutoPerThreadSystem pts(msf.get());
+  IFTLLVM(pts.error_code());
+
   CComPtr<Session> pSession = Session::Alloc(DxcGetThreadMallocNoRef());
   IFROOM(pSession.p);
   pSession->Init(m_context, m_module, m_finder);

lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp

@@ -483,6 +483,9 @@ void VariableRegisters::PopulateAllocaMap(
       PopulateAllocaMap(
           DerivedTy->getBaseType().resolve(EmptyMap));
       return;
+    case llvm::dwarf::DW_TAG_subroutine_type:
+        //ignore member functions.
+      return;
     }
   }
   else if (auto *CompositeTy = llvm::dyn_cast<llvm::DICompositeType>(Ty))
@@ -647,7 +650,6 @@ static bool SortMembers(
     std::map<OffsetInBits, llvm::DIDerivedType*> *SortedMembers
 )
 {
-  const llvm::DITypeIdentifierMap EmptyMap;
   for (auto *Element : Ty->getElements())
   {
     switch (Element->getTag())
@@ -664,7 +666,15 @@ static bool SortMembers(
         }
         break;
       }
-      // FALLTHROUGH
+      assert(!"member is not a Member");
+      return false;
+    }
+    case llvm::dwarf::DW_TAG_subprogram: {
+      if (auto *SubProgram = llvm::dyn_cast<llvm::DISubprogram>(Element)) {
+        break;
+      }
+      assert(!"DISubprogram not understood");
+      return false;
     }
     default:
       assert(!"Unhandled field type in DIStructType");

tools/clang/unittests/HLSL/PixTest.cpp

@@ -204,6 +204,7 @@ public:
   TEST_METHOD(PixStructAnnotation_SequentialFloatN)
   TEST_METHOD(PixStructAnnotation_EmbeddedFloatN)
   TEST_METHOD(PixStructAnnotation_Matrix)
+  TEST_METHOD(PixStructAnnotation_MemberFunction)
   TEST_METHOD(PixStructAnnotation_BigMess)
 
   dxc::DxcDllSupport m_dllSupport;
@@ -1767,61 +1768,65 @@ PixTest::TestableResults PixTest::TestStructAnnotationCase(const char* hlsl)
       {
         llvm::Value* Address = dbgDeclare->getAddress();
         auto* AddressAsAlloca = llvm::dyn_cast<llvm::AllocaInst>(Address);
-        auto* Expression = dbgDeclare->getExpression();
-
-        std::unique_ptr<dxil_debug_info::MemberIterator> iterator = dxil_debug_info::CreateMemberIterator(
-          dbgDeclare,
-          moduleEtc.GetDxilModule().GetModule()->getDataLayout(),
-          AddressAsAlloca,
-          Expression);
-
-        unsigned int startingBit = 0;
-        unsigned int coveredBits = 0;
-        unsigned int memberIndex = 0;
-        while (iterator->Next(&memberIndex))
+        if (AddressAsAlloca != nullptr)
         {
-          if (memberIndex == 0)
-          {
-            startingBit = iterator->OffsetInBits(memberIndex);
-            coveredBits = iterator->SizeInBits(memberIndex);
-          }
-          else
-          {
-            coveredBits = std::max<unsigned int>( coveredBits, iterator->OffsetInBits(memberIndex) + iterator->SizeInBits(memberIndex));
-          }
-        }
-
-        // memberIndex is now the count of members in this aggregate type
-        ret.OffsetAndSizes.push_back({ memberIndex, startingBit, coveredBits });
-
-        // Use this independent count of number of struct members to test the 
-        // function that operates on the alloca type:
-        llvm::Type *pAllocaTy = AddressAsAlloca->getType()->getElementType();
-        if (auto *AT = llvm::dyn_cast<llvm::ArrayType>(pAllocaTy))
-        {
-          // This is the case where a struct is passed to a function, and in 
-          // these tests there should be only one struct behind the pointer.
-          VERIFY_ARE_EQUAL(AT->getNumElements(), 1);
-          pAllocaTy = AT->getArrayElementType();
-        }
-
-        if (auto* ST = llvm::dyn_cast<llvm::StructType>(pAllocaTy))
-        {
-          uint32_t countOfMembers = CountStructMembers(ST);
-          // memberIndex might be greater, because the fragment iterator also includes contained derived types as
-          // fragments, in addition to the members of that contained derived types. CountStructMembers only counts
-          // the leaf-node types.
-          VERIFY_ARE_EQUAL(countOfMembers, memberIndex);
-        }
-        else if (pAllocaTy->isFloatingPointTy() || pAllocaTy->isIntegerTy())
-        {
-          // If there's only one member in the struct in the pass-to-function (by pointer)
-          // case, then the underlying type will have been reduced to the contained type.
-          VERIFY_ARE_EQUAL(1, memberIndex);
-        }
-        else
-        {
-          VERIFY_IS_TRUE(false);
+            auto* Expression = dbgDeclare->getExpression();
+
+            std::unique_ptr<dxil_debug_info::MemberIterator> iterator = dxil_debug_info::CreateMemberIterator(
+                dbgDeclare,
+                moduleEtc.GetDxilModule().GetModule()->getDataLayout(),
+                AddressAsAlloca,
+                Expression);
+
+            unsigned int startingBit = 0;
+            unsigned int coveredBits = 0;
+            unsigned int memberIndex = 0;
+            unsigned int memberCount = 0;
+            while (iterator->Next(&memberIndex))
+            {
+                memberCount++;
+                if (memberIndex == 0)
+                {
+                    startingBit = iterator->OffsetInBits(memberIndex);
+                    coveredBits = iterator->SizeInBits(memberIndex);
+                }
+                else
+                {
+                    coveredBits = std::max<unsigned int>(coveredBits, iterator->OffsetInBits(memberIndex) + iterator->SizeInBits(memberIndex));
+                }
+            }
+
+            ret.OffsetAndSizes.push_back({ memberCount, startingBit, coveredBits });
+
+            // Use this independent count of number of struct members to test the 
+            // function that operates on the alloca type:
+            llvm::Type* pAllocaTy = AddressAsAlloca->getType()->getElementType();
+            if (auto* AT = llvm::dyn_cast<llvm::ArrayType>(pAllocaTy))
+            {
+                // This is the case where a struct is passed to a function, and in 
+                // these tests there should be only one struct behind the pointer.
+                VERIFY_ARE_EQUAL(AT->getNumElements(), 1);
+                pAllocaTy = AT->getArrayElementType();
+            }
+
+            if (auto* ST = llvm::dyn_cast<llvm::StructType>(pAllocaTy))
+            {
+                uint32_t countOfMembers = CountStructMembers(ST);
+                // memberIndex might be greater, because the fragment iterator also includes contained derived types as
+                // fragments, in addition to the members of that contained derived types. CountStructMembers only counts
+                // the leaf-node types.
+                VERIFY_ARE_EQUAL(countOfMembers, memberCount);
+            }
+            else if (pAllocaTy->isFloatingPointTy() || pAllocaTy->isIntegerTy())
+            {
+                // If there's only one member in the struct in the pass-to-function (by pointer)
+                // case, then the underlying type will have been reduced to the contained type.
+                VERIFY_ARE_EQUAL(1, memberCount);
+            }
+            else
+            {
+                VERIFY_IS_TRUE(false);
+            }
         }
       }
     }
@@ -1892,7 +1897,7 @@ PixTest::TestableResults PixTest::TestStructAnnotationCase(const char* hlsl)
 
 void PixTest::ValidateAllocaWrite(std::vector<AllocaWrite> const &allocaWrites,
                                   size_t index, const char *name) {
-  VERIFY_ARE_EQUAL(index, allocaWrites[index].index);
+  VERIFY_ARE_EQUAL(index, allocaWrites[index].regBase + allocaWrites[index].index);
 #if DBG
   // Compilation may add a prefix to the struct member name:
   VERIFY_IS_TRUE(0 == strncmp(name, allocaWrites[index].memberName.c_str(), strlen(name)));
@@ -2295,6 +2300,123 @@ void main()
 
 }
 
+TEST_F(PixTest, PixStructAnnotation_MemberFunction) {
+  const char *hlsl = R"(
+
+RWStructuredBuffer<float> floatRWUAV: register(u0);
+
+struct smallPayload
+{
+    int i;
+};
+
+float2 signNotZero(float2 v)
+{
+ return (v > 0.0f ? float(1).xx : float(-1).xx);
+}
+
+float2 unpackUnorm2(uint packed)
+{
+ return (1.0 / 65535.0) * float2((packed >> 16) & 0xffff, packed & 0xffff);
+}
+
+float3 unpackOctahedralSnorm(float2 e)
+{
+ float3 v = float3(e.xy, 1.0f - abs(e.x) - abs(e.y));
+ if (v.z < 0.0f) v.xy = (1.0f - abs(v.yx)) * signNotZero(v.xy);
+ return normalize(v);
+}
+
+float3 unpackOctahedralUnorm(float2 e)
+{
+ return unpackOctahedralSnorm(e * 2.0f - 1.0f);
+}
+
+float2 unpackHalf2(uint packed)
+{
+ return float2(f16tof32(packed >> 16), f16tof32(packed & 0xffff));
+}
+
+struct Gbuffer
+{
+	float3 worldNormal;
+	float3 objectNormal; //offset:12
+	
+	float linearZ; //24
+	float prevLinearZ; //28
+	
+	
+	float fwidthLinearZ; //32
+	float fwidthObjectNormal; //36
+	
+	
+	uint materialType; //40
+	uint2 materialParams0; //44
+	uint4 materialParams1; //52  <--------- this is the variable that's being covered twice (52*8 = 416 416)
+	
+	uint instanceId;  //68  <------- and there's one dword left over, as expected
+	
+	
+	void load(int2 pixelPos, Texture2DArray<uint4> gbTex)
+	{
+	uint4 data0 = gbTex.Load(int4(pixelPos, 0, 0));
+	uint4 data1 = gbTex.Load(int4(pixelPos, 1, 0));
+	uint4 data2 = gbTex.Load(int4(pixelPos, 2, 0));
+	
+	
+	worldNormal = unpackOctahedralUnorm(unpackUnorm2(data0.x));
+	linearZ = f16tof32((data0.y >> 8) & 0xffff);
+	materialType = (data0.y & 0xff);
+	materialParams0 = data0.zw;
+	
+	
+	materialParams1 = data1.xyzw;
+	
+	
+	instanceId = data2.x;
+	prevLinearZ = asfloat(data2.y);
+	objectNormal = unpackOctahedralUnorm(unpackUnorm2(data2.z));
+	float2 fwidth = unpackHalf2(data2.w);
+	fwidthLinearZ = fwidth.x;
+	fwidthObjectNormal = fwidth.y;
+	}
+};
+
+Gbuffer loadGbuffer(int2 pixelPos, Texture2DArray<uint4> gbTex)
+{
+	Gbuffer output;
+	output.load(pixelPos, gbTex);
+	return output;
+}
+
+Texture2DArray<uint4> g_gbuffer : register(t0, space0);
+
+[numthreads(1, 1, 1)]
+void main()
+{	
+	const Gbuffer gbuffer = loadGbuffer(int2(0,0), g_gbuffer);
+    smallPayload p;
+    p.i = gbuffer.materialParams1.x + gbuffer.materialParams1.y + gbuffer.materialParams1.z + gbuffer.materialParams1.w;
+    DispatchMesh(1, 1, 1, p);
+}
+
+
+)";
+
+  auto Testables = TestStructAnnotationCase(hlsl);
+  // Can't test member iterator until dbg.declare instructions are emitted when structs
+  // contain pointers-to-pointers
+  
+  // Can't validate # of writes: rel and dbg are different
+  //VERIFY_ARE_EQUAL(43, Testables.AllocaWrites.size());
+
+  // Can't test individual writes until struct member names are returned:
+  //for (int i = 0; i < 51; ++i)
+  //{
+  //  ValidateAllocaWrite(Testables.AllocaWrites, i, "");
+  //}
+}
+
 TEST_F(PixTest, PixStructAnnotation_BigMess) {
   if (m_ver.SkipDxilVersion(1, 5)) return;