[HLSL] Add codegen for accessing resource members of a struct (2nd merge attempt) (llvm#193584)

hekota · web-flow · commit 87a8d40fdd44 · 2026-04-23T09:20:36.000-07:00
Any expression that accesses a resource or resource array member of a global struct instance must be during codegen replaced by an access of the corresponding implicit global resource variable. When codegen encounters a `MemberExpr` of a resource type, it traverses the AST to locate the parent struct declaration, building the expected global resource variable name along the way. If the parent declaration is a non-static global struct instance, codegen searches its `HLSLAssociatedResourceDeclAttr` attributes to locate the matching global resource variable and then generates IR code to access the resource global in place of the member access. Fixes llvm#182989 This is the second try to land this. The [first one](llvm#187127 with llvm#188792 and both PRs had to be reverted. No updates needed to this change. I synced with @inbelic and we agreed that this one should go in first.
diff --git a/clang/include/clang/AST/HLSLResource.h b/clang/include/clang/AST/HLSLResource.h
@@ -109,6 +109,21 @@ inline uint32_t getResourceDimensions(llvm::dxil::ResourceDimension Dim) {
   llvm_unreachable("Unhandled llvm::dxil::ResourceDimension enum.");
 }
 
+// Returns true if the second field of the record is a counter resource handle
+inline bool hasCounterHandle(const CXXRecordDecl *RD) {
+  if (RD->field_empty())
+    return false;
+  auto It = std::next(RD->field_begin());
+  if (It == RD->field_end())
+    return false;
+  const FieldDecl *SecondField = *It;
+  if (const auto *ResTy =
+          SecondField->getType()->getAs<HLSLAttributedResourceType>()) {
+    return ResTy->getAttrs().IsCounter;
+  }
+  return false;
+}
+
 // Helper class for building a name of a global resource variable that
 // gets created for a resource embedded in a struct or class. This will
 // also be used from CodeGen to build a name that matches the resource
@@ -128,6 +143,7 @@ class EmbeddedResourceNameBuilder {
   void pushName(llvm::StringRef N) { pushName(N, FieldDelim); }
   void pushBaseName(llvm::StringRef N);
   void pushArrayIndex(uint64_t Index);
+  void pushBaseNameHierarchy(CXXRecordDecl *DerivedRD, CXXRecordDecl *BaseRD);
 
   void pop() {
     assert(!Offsets.empty() && "no name to pop");
diff --git a/clang/lib/AST/HLSLResource.cpp b/clang/lib/AST/HLSLResource.cpp
@@ -41,5 +41,20 @@ void EmbeddedResourceNameBuilder::pushArrayIndex(uint64_t Index) {
   OS << Index;
 }
 
+void EmbeddedResourceNameBuilder::pushBaseNameHierarchy(
+    CXXRecordDecl *DerivedRD, CXXRecordDecl *BaseRD) {
+  assert(BaseRD != DerivedRD && DerivedRD->isDerivedFrom(BaseRD));
+  Offsets.push_back(Name.size());
+  Name.append(FieldDelim);
+  while (BaseRD != DerivedRD) {
+    assert(DerivedRD->getNumBases() == 1 &&
+           "HLSL does not support multiple inheritance");
+    DerivedRD = DerivedRD->bases_begin()->getType()->getAsCXXRecordDecl();
+    assert(DerivedRD && "base class not found");
+    Name.append(DerivedRD->getName());
+    Name.append(BaseClassDelim);
+  }
+}
+
 } // namespace hlsl
 } // namespace clang
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
@@ -5545,10 +5545,18 @@ LValue CodeGenFunction::EmitMemberExpr(const MemberExpr *E) {
     EmitIgnoredExpr(E->getBase());
     return EmitDeclRefLValue(DRE);
   }
-  if (getLangOpts().HLSL &&
-      E->getType().getAddressSpace() == LangAS::hlsl_constant) {
-    // We have an HLSL buffer - emit using HLSL's layout rules.
-    return CGM.getHLSLRuntime().emitBufferMemberExpr(*this, E);
+
+  if (getLangOpts().HLSL) {
+    QualType QT = E->getType();
+    if (QT.getAddressSpace() == LangAS::hlsl_constant)
+      return CGM.getHLSLRuntime().emitBufferMemberExpr(*this, E);
+
+    if (QT->isHLSLResourceRecord() || QT->isHLSLResourceRecordArray()) {
+      std::optional<LValue> LV;
+      LV = CGM.getHLSLRuntime().emitResourceMemberExpr(*this, E);
+      if (LV.has_value())
+        return *LV;
+    }
   }
 
   Expr *BaseExpr = E->getBase();
diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -96,24 +96,120 @@ void addRootSignatureMD(llvm::dxbc::RootSignatureVersion RootSigVer,
   RootSignatureValMD->addOperand(MDVals);
 }
 
+// Given a MemberExpr of a resource or resource array type, find the parent
+// VarDecl of the struct or class instance that contains this resource and
+// build the full resource name based on the member access path.
+//
+// For example, for a member access like "myStructArray[0].memberA",
+// this function will find the VarDecl of "myStructArray" and use the
+// EmbeddedResourceNameBuilder to build the resource name
+// "myStructArray.0.memberA".
+static const VarDecl *findStructResourceParentDeclAndBuildName(
+    const MemberExpr *ME, EmbeddedResourceNameBuilder &NameBuilder) {
+
+  SmallVector<const Expr *> WorkList;
+  const VarDecl *VD = nullptr;
+  const Expr *E = ME;
+
+  for (;;) {
+    if (const auto *DRE = dyn_cast<DeclRefExpr>(E)) {
+      assert(isa<VarDecl>(DRE->getDecl()) &&
+             "member expr base is not a var decl");
+      VD = cast<VarDecl>(DRE->getDecl());
+      NameBuilder.pushName(VD->getName());
+      break;
+    }
+
+    WorkList.push_back(E);
+    if (const auto *MExp = dyn_cast<MemberExpr>(E))
+      E = MExp->getBase();
+    else if (const auto *ICE = dyn_cast<ImplicitCastExpr>(E))
+      E = ICE->getSubExpr();
+    else if (const auto *ASE = dyn_cast<ArraySubscriptExpr>(E))
+      E = ASE->getBase();
+    else if (isa<CXXThisExpr>(E))
+      // Resource member access on "this" pointer not yet implemented
+      // (llvm/llvm-project#190299)
+      return nullptr;
+    else
+      llvm_unreachable("unexpected expr type in resource member access");
+
+    assert(E && "expected valid expression");
+  }
+
+  while (!WorkList.empty()) {
+    E = WorkList.pop_back_val();
+    if (const auto *ME = dyn_cast<MemberExpr>(E)) {
+      NameBuilder.pushName(
+          ME->getMemberNameInfo().getName().getAsIdentifierInfo()->getName());
+    } else if (const auto *ICE = dyn_cast<ImplicitCastExpr>(E)) {
+      if (ICE->getCastKind() == CK_UncheckedDerivedToBase) {
+        CXXRecordDecl *DerivedRD =
+            ICE->getSubExpr()->getType()->getAsCXXRecordDecl();
+        CXXRecordDecl *BaseRD = ICE->getType()->getAsCXXRecordDecl();
+        NameBuilder.pushBaseNameHierarchy(DerivedRD, BaseRD);
+      }
+    } else if (const auto *ASE = dyn_cast<ArraySubscriptExpr>(E)) {
+      const Expr *IdxExpr = ASE->getIdx();
+      std::optional<llvm::APSInt> Value =
+          IdxExpr->getIntegerConstantExpr(VD->getASTContext());
+      assert(Value &&
+             "expected constant index in struct with resource array access");
+      NameBuilder.pushArrayIndex(Value->getZExtValue());
+    } else {
+      llvm_unreachable("unexpected expr type in resource member access");
+    }
+  }
+  return VD;
+}
+
+// Given a MemberExpr of a resource or resource array type, find the
+// corresponding global resource declaration associated with the owning struct
+// or class instance via HLSLAssociatedResourceDeclAttr.
+static const VarDecl *
+findAssociatedResourceDeclForStruct(ASTContext &AST, const MemberExpr *ME) {
+
+  EmbeddedResourceNameBuilder NameBuilder;
+  const VarDecl *ParentVD =
+      findStructResourceParentDeclAndBuildName(ME, NameBuilder);
+  if (!ParentVD)
+    return nullptr;
+
+  if (!ParentVD->hasGlobalStorage())
+    return nullptr;
+
+  IdentifierInfo *II = NameBuilder.getNameAsIdentifier(AST);
+  for (const Attr *A : ParentVD->getAttrs()) {
+    if (const auto *ADA = dyn_cast<HLSLAssociatedResourceDeclAttr>(A)) {
+      VarDecl *AssocResVD = ADA->getResDecl();
+      if (AssocResVD->getIdentifier() == II)
+        return AssocResVD;
+    }
+  }
+  return nullptr;
+}
+
 // Find array variable declaration from DeclRef expression
-static const ValueDecl *getArrayDecl(const Expr *E) {
-  if (const DeclRefExpr *DRE =
-          dyn_cast_or_null<DeclRefExpr>(E->IgnoreImpCasts()))
+static const ValueDecl *getArrayDecl(ASTContext &AST, const Expr *E) {
+  E = E->IgnoreImpCasts();
+  if (const auto *DRE = dyn_cast_or_null<DeclRefExpr>(E))
     return DRE->getDecl();
+  if (isa<MemberExpr>(E))
+    return findAssociatedResourceDeclForStruct(AST, cast<MemberExpr>(E));
   return nullptr;
 }
 
 // Find array variable declaration from nested array subscript AST nodes
-static const ValueDecl *getArrayDecl(const ArraySubscriptExpr *ASE) {
+static const ValueDecl *getArrayDecl(ASTContext &AST,
+                                     const ArraySubscriptExpr *ASE) {
   const Expr *E = nullptr;
   while (ASE != nullptr) {
     E = ASE->getBase()->IgnoreImpCasts();
     if (!E)
       return nullptr;
     ASE = dyn_cast<ArraySubscriptExpr>(E);
   }
-  return getArrayDecl(E);
+  return getArrayDecl(AST, E);
 }
 
 // Get the total size of the array, or 0 if the array is unbounded.
@@ -152,6 +248,10 @@ static CXXMethodDecl *lookupResourceInitMethodAndSetupArgs(
   Value *NameStr = buildNameForResource(Name, CGM);
   Value *Space = llvm::ConstantInt::get(CGM.IntTy, Binding.getSpace());
 
+  bool HasCounter = hasCounterHandle(ResourceDecl);
+  assert((!HasCounter || Binding.hasCounterImplicitOrderID()) &&
+         "resources with counter handle must have a binding with counter "
+         "implicit order ID");
   if (Binding.isExplicit()) {
     // explicit binding
     auto *RegSlot = llvm::ConstantInt::get(CGM.IntTy, Binding.getSlot());
@@ -174,7 +274,7 @@ static CXXMethodDecl *lookupResourceInitMethodAndSetupArgs(
   Args.add(RValue::get(Range), AST.IntTy);
   Args.add(RValue::get(Index), AST.UnsignedIntTy);
   Args.add(RValue::get(NameStr), AST.getPointerType(AST.CharTy.withConst()));
-  if (Binding.hasCounterImplicitOrderID()) {
+  if (HasCounter) {
     uint32_t CounterBinding = Binding.getCounterImplicitOrderID();
     auto *CounterOrderID = llvm::ConstantInt::get(CGM.IntTy, CounterBinding);
     Args.add(RValue::get(CounterOrderID), AST.UnsignedIntTy);
@@ -1243,8 +1343,8 @@ std::optional<LValue> CGHLSLRuntime::emitResourceArraySubscriptExpr(
   // Let clang codegen handle local and static resource array subscripts,
   // or when the subscript references on opaque expression (as part of
   // ArrayInitLoopExpr AST node).
-  const VarDecl *ArrayDecl =
-      dyn_cast_or_null<VarDecl>(getArrayDecl(ArraySubsExpr));
+  const VarDecl *ArrayDecl = dyn_cast_or_null<VarDecl>(
+      getArrayDecl(CGF.CGM.getContext(), ArraySubsExpr));
   if (!ArrayDecl || !ArrayDecl->hasGlobalStorage() ||
       ArrayDecl->getStorageClass() == SC_Static)
     return std::nullopt;
@@ -1309,11 +1409,15 @@ std::optional<LValue> CGHLSLRuntime::emitResourceArraySubscriptExpr(
         CGF.CGM, ResourceTy->getAsCXXRecordDecl(), Range, Index,
         ArrayDecl->getName(), Binding, Args);
 
-    if (!CreateMethod)
+    if (!CreateMethod) {
       // This can happen if someone creates an array of structs that looks like
       // an HLSL resource record array but it does not have the required static
       // create method. No binding will be generated for it.
+      assert(!ResourceTy->getAsCXXRecordDecl()->isImplicit() &&
+             "create method lookup should always succeed for built-in resource "
+             "records");
       return std::nullopt;
+    }
 
     callResourceInitMethod(CGF, CreateMethod, Args, ValueSlot.getAddress());
 
@@ -1341,7 +1445,8 @@ bool CGHLSLRuntime::emitResourceArrayCopy(LValue &LHS, Expr *RHSExpr,
   assert(ResultTy->isHLSLResourceRecordArray() && "expected resource array");
 
   // Let Clang codegen handle local and static resource array copies.
-  const VarDecl *ArrayDecl = dyn_cast_or_null<VarDecl>(getArrayDecl(RHSExpr));
+  const VarDecl *ArrayDecl =
+      dyn_cast_or_null<VarDecl>(getArrayDecl(CGF.CGM.getContext(), RHSExpr));
   if (!ArrayDecl || !ArrayDecl->hasGlobalStorage() ||
       ArrayDecl->getStorageClass() == SC_Static)
     return false;
@@ -1464,6 +1569,37 @@ std::optional<LValue> CGHLSLRuntime::emitBufferArraySubscriptExpr(
   return CGF.MakeAddrLValue(Addr, E->getType(), EltBaseInfo, EltTBAAInfo);
 }
 
+std::optional<LValue>
+CGHLSLRuntime::emitResourceMemberExpr(CodeGenFunction &CGF,
+                                      const MemberExpr *ME) {
+  assert((ME->getType()->isHLSLResourceRecord() ||
+          ME->getType()->isHLSLResourceRecordArray()) &&
+         "expected resource member expression");
+
+  if (ME->getType()->isHLSLResourceRecordArray()) {
+    // FIXME: Handle member access of the whole array of resources
+    // (llvm/llvm-project#187087). Access to individual resource array elements
+    // is already handled in emitResourceArraySubscriptExpr.
+    return std::nullopt;
+  }
+
+  const VarDecl *ResourceVD =
+      findAssociatedResourceDeclForStruct(CGF.CGM.getContext(), ME);
+  if (!ResourceVD)
+    return std::nullopt;
+
+  GlobalVariable *ResGV =
+      cast<GlobalVariable>(CGM.GetAddrOfGlobalVar(ResourceVD));
+  const DataLayout &DL = CGM.getDataLayout();
+  llvm::Type *Ty = ResGV->getValueType();
+  CharUnits Align = CharUnits::fromQuantity(DL.getABITypeAlign(Ty));
+  Address Addr = Address(ResGV, Ty, Align);
+  LValue LV = LValue::MakeAddr(Addr, ME->getType(), CGM.getContext(),
+                               LValueBaseInfo(AlignmentSource::Type),
+                               CGM.getTBAAAccessInfo(ME->getType()));
+  return LV;
+}
+
 namespace {
 /// Utility for emitting copies following the HLSL buffer layout rules (ie,
 /// copying out of a cbuffer).
diff --git a/clang/lib/CodeGen/CGHLSLRuntime.h b/clang/lib/CodeGen/CGHLSLRuntime.h
@@ -304,6 +304,8 @@ class CGHLSLRuntime {
                       QualType CType);
 
   LValue emitBufferMemberExpr(CodeGenFunction &CGF, const MemberExpr *E);
+  std::optional<LValue> emitResourceMemberExpr(CodeGenFunction &CGF,
+                                               const MemberExpr *E);
 
 private:
   void emitBufferGlobalsAndMetadata(const HLSLBufferDecl *BufDecl,
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
@@ -1489,20 +1489,6 @@ static CXXMethodDecl *lookupMethod(Sema &S, CXXRecordDecl *RecordDecl,
 
 } // end anonymous namespace
 
-static bool hasCounterHandle(const CXXRecordDecl *RD) {
-  if (RD->field_empty())
-    return false;
-  auto It = std::next(RD->field_begin());
-  if (It == RD->field_end())
-    return false;
-  const FieldDecl *SecondField = *It;
-  if (const auto *ResTy =
-          SecondField->getType()->getAs<HLSLAttributedResourceType>()) {
-    return ResTy->getAttrs().IsCounter;
-  }
-  return false;
-}
-
 bool SemaHLSL::handleRootSignatureElements(
     ArrayRef<hlsl::RootSignatureElement> Elements) {
   // Define some common error handling functions
@@ -5060,7 +5046,7 @@ class StructBindingContext {
   // Creates a binding attribute for a resource based on the gathered attributes
   // and the required register type and range.
   Attr *createBindingAttr(SemaHLSL &S, ASTContext &AST, RegisterType RegType,
-                          unsigned Range) {
+                          unsigned Range, bool HasCounter) {
     assert(static_cast<unsigned>(RegType) < 4 && "unexpected register type");
 
     if (VkBindingAttr) {
@@ -5095,6 +5081,9 @@ class StructBindingContext {
                           RBA ? RBA->getSpaceNumber() : 0);
       NewAttr->setImplicitBindingOrderID(S.getNextImplicitBindingOrderID());
     }
+    if (HasCounter)
+      NewAttr->setImplicitCounterBindingOrderID(
+          S.getNextImplicitBindingOrderID());
     return NewAttr;
   }
 };
@@ -5116,18 +5105,20 @@ static void createGlobalResourceDeclForStruct(
       VarDecl::Create(AST, DC, Loc, Loc, Id, ResTy, nullptr, SC_None);
 
   unsigned Range = 1;
-  const HLSLAttributedResourceType *ResHandleTy = nullptr;
-  if (const auto *AT = dyn_cast<ArrayType>(ResTy.getTypePtr())) {
+  const Type *SingleResTy = ResTy.getTypePtr()->getUnqualifiedDesugaredType();
+  while (const auto *AT = dyn_cast<ArrayType>(SingleResTy)) {
     const auto *CAT = dyn_cast<ConstantArrayType>(AT);
-    Range = CAT ? CAT->getSize().getZExtValue() : 0;
-    ResHandleTy = getResourceArrayHandleType(ResTy);
-  } else {
-    ResHandleTy = HLSLAttributedResourceType::findHandleTypeOnResource(
-        ResTy.getTypePtr());
+    Range = CAT ? (Range * CAT->getSize().getZExtValue()) : 0;
+    SingleResTy =
+        AT->getArrayElementTypeNoTypeQual()->getUnqualifiedDesugaredType();
   }
+  const HLSLAttributedResourceType *ResHandleTy =
+      HLSLAttributedResourceType::findHandleTypeOnResource(SingleResTy);
+
   // Add a binding attribute to the global resource declaration.
+  bool HasCounter = hasCounterHandle(SingleResTy->getAsCXXRecordDecl());
   Attr *BindingAttr = BindingCtx.createBindingAttr(
-      S.HLSL(), AST, getRegisterType(ResHandleTy), Range);
+      S.HLSL(), AST, getRegisterType(ResHandleTy), Range, HasCounter);
   ResDecl->addAttr(BindingAttr);
   ResDecl->addAttr(InternalLinkageAttr::CreateImplicit(AST));
   ResDecl->setImplicit();
@@ -5366,11 +5357,15 @@ bool SemaHLSL::initGlobalResourceDecl(VarDecl *VD) {
   CreateMethod =
       lookupMethod(SemaRef, ResourceDecl, CreateMethodName, VD->getLocation());
 
-  if (!CreateMethod)
+  if (!CreateMethod) {
     // This can happen if someone creates a struct that looks like an HLSL
     // resource record but does not have the required static create method.
     // No binding will be generated for it.
+    assert(!ResourceDecl->isImplicit() &&
+           "create method lookup should always succeed for built-in resource "
+           "records");
     return false;
+  }
 
   if (Binding.isExplicit()) {
     IntegerLiteral *RegSlot =
diff --git a/clang/test/CodeGenHLSL/resources/resources-in-structs-array.hlsl b/clang/test/CodeGenHLSL/resources/resources-in-structs-array.hlsl
diff --git a/clang/test/CodeGenHLSL/resources/resources-in-structs-inheritance.hlsl b/clang/test/CodeGenHLSL/resources/resources-in-structs-inheritance.hlsl
diff --git a/clang/test/CodeGenHLSL/resources/resources-in-structs.hlsl b/clang/test/CodeGenHLSL/resources/resources-in-structs.hlsl
