[HLSL] Wrap offset info into a dedicated type. NFC

Rather than using a nullable SmallVector, use a wrapper class for offset
info. This simplifies places that need to handle whether or not there's
any offset information.

Author: bogner

clang/lib/CodeGen/CGHLSLRuntime.cpp

@@ -261,12 +261,12 @@ static std::optional<llvm::Value *> initializeLocalResourceArray(
 
 llvm::Type *
 CGHLSLRuntime::convertHLSLSpecificType(const Type *T,
-                                       SmallVector<int32_t> *Packoffsets) {
+                                       const CGHLSLOffsetInfo &OffsetInfo) {
   assert(T->isHLSLSpecificType() && "Not an HLSL specific type!");
 
   // Check if the target has a specific translation for this type first.
   if (llvm::Type *TargetTy =
-          CGM.getTargetCodeGenInfo().getHLSLType(CGM, T, Packoffsets))
+          CGM.getTargetCodeGenInfo().getHLSLType(CGM, T, OffsetInfo))
     return TargetTy;
 
   llvm_unreachable("Generic handling of HLSL types is not supported.");
@@ -357,25 +357,14 @@ createBufferHandleType(const HLSLBufferDecl *BufDecl) {
   return cast<HLSLAttributedResourceType>(QT.getTypePtr());
 }
 
-// Iterates over all declarations in the HLSL buffer and based on the
-// packoffset or register(c#) annotations it fills outs the Layout
-// vector with the user-specified layout offsets.
-// The buffer offsets can be specified 2 ways:
-// 1. declarations in cbuffer {} block can have a packoffset annotation
-//    (translates to HLSLPackOffsetAttr)
-// 2. default constant buffer declarations at global scope can have
-//    register(c#) annotations (translates to HLSLResourceBindingAttr with
-//    RegisterType::C)
-// It is not guaranteed that all declarations in a buffer have an annotation.
-// For those where it is not specified a -1 value is added to the Layout
-// vector. In the final layout these declarations will be placed at the end
-// of the HLSL buffer after all of the elements with specified offset.
-static void fillPackoffsetLayout(const HLSLBufferDecl *BufDecl,
-                                 SmallVector<int32_t> &Layout) {
-  assert(Layout.empty() && "expected empty vector for layout");
-  assert(BufDecl->hasValidPackoffset());
+CGHLSLOffsetInfo CGHLSLOffsetInfo::fromDecl(const HLSLBufferDecl &BufDecl) {
+  CGHLSLOffsetInfo Result;
 
-  for (Decl *D : BufDecl->buffer_decls()) {
+  // If we don't have packoffset info, just return an empty result.
+  if (!BufDecl.hasValidPackoffset())
+    return Result;
+
+  for (Decl *D : BufDecl.buffer_decls()) {
     if (isa<CXXRecordDecl, EmptyDecl>(D) || isa<FunctionDecl>(D)) {
       continue;
     }
@@ -384,11 +373,11 @@ static void fillPackoffsetLayout(const HLSLBufferDecl *BufDecl,
       continue;
 
     if (!VD->hasAttrs()) {
-      Layout.push_back(-1);
+      Result.Offsets.push_back(Unspecified);
       continue;
     }
 
-    int32_t Offset = -1;
+    uint32_t Offset = Unspecified;
     for (auto *Attr : VD->getAttrs()) {
       if (auto *POA = dyn_cast<HLSLPackOffsetAttr>(Attr)) {
         Offset = POA->getOffsetInBytes();
@@ -401,8 +390,9 @@ static void fillPackoffsetLayout(const HLSLBufferDecl *BufDecl,
         break;
       }
     }
-    Layout.push_back(Offset);
+    Result.Offsets.push_back(Offset);
   }
+  return Result;
 }
 
 // Codegen for HLSLBufferDecl
@@ -419,13 +409,9 @@ void CGHLSLRuntime::addBuffer(const HLSLBufferDecl *BufDecl) {
     return;
 
   // create global variable for the constant buffer
-  SmallVector<int32_t> Layout;
-  if (BufDecl->hasValidPackoffset())
-    fillPackoffsetLayout(BufDecl, Layout);
-
-  llvm::TargetExtType *TargetTy =
-      cast<llvm::TargetExtType>(convertHLSLSpecificType(
-          ResHandleTy, BufDecl->hasValidPackoffset() ? &Layout : nullptr));
+  CGHLSLOffsetInfo OffsetInfo = CGHLSLOffsetInfo::fromDecl(*BufDecl);
+  llvm::TargetExtType *TargetTy = cast<llvm::TargetExtType>(
+      convertHLSLSpecificType(ResHandleTy, OffsetInfo));
   llvm::GlobalVariable *BufGV = new GlobalVariable(
       TargetTy, /*isConstant*/ false,
       GlobalValue::LinkageTypes::ExternalLinkage, PoisonValue::get(TargetTy),

clang/lib/CodeGen/CGHLSLRuntime.h

@@ -81,6 +81,33 @@ class CodeGenModule;
 class CodeGenFunction;
 class LValue;
 
+class CGHLSLOffsetInfo {
+  SmallVector<uint32_t> Offsets;
+
+public:
+  static const uint32_t Unspecified = ~0U;
+
+  /// Iterates over all declarations in the HLSL buffer and based on the
+  /// packoffset or register(c#) annotations it fills outs the Offsets vector
+  /// with the user-specified layout offsets. The buffer offsets can be
+  /// specified 2 ways: 1. declarations in cbuffer {} block can have a
+  /// packoffset annotation (translates to HLSLPackOffsetAttr) 2. default
+  /// constant buffer declarations at global scope can have register(c#)
+  /// annotations (translates to HLSLResourceBindingAttr with RegisterType::C)
+  /// It is not guaranteed that all declarations in a buffer have an annotation.
+  /// For those where it is not specified a `~0U` value is added to the Offsets
+  /// vector. In the final layout these declarations will be placed at the end
+  /// of the HLSL buffer after all of the elements with specified offset.
+  static CGHLSLOffsetInfo fromDecl(const HLSLBufferDecl &BufDecl);
+
+  /// Get the given offset, or `~0U` if there is no offset for the member.
+  uint32_t operator[](size_t I) const {
+    if (Offsets.empty())
+      return Unspecified;
+    return Offsets[I];
+  }
+};
+
 class CGHLSLRuntime {
 public:
   //===----------------------------------------------------------------------===//
@@ -167,9 +194,11 @@ class CGHLSLRuntime {
   CGHLSLRuntime(CodeGenModule &CGM) : CGM(CGM) {}
   virtual ~CGHLSLRuntime() {}
 
-  llvm::Type *
-  convertHLSLSpecificType(const Type *T,
-                          SmallVector<int32_t> *Packoffsets = nullptr);
+  llvm::Type *convertHLSLSpecificType(const Type *T,
+                                      const CGHLSLOffsetInfo &OffsetInfo);
+  llvm::Type *convertHLSLSpecificType(const Type *T) {
+    return convertHLSLSpecificType(T, CGHLSLOffsetInfo());
+  }
 
   void generateGlobalCtorDtorCalls();
 

clang/lib/CodeGen/HLSLBufferLayoutBuilder.cpp

@@ -66,8 +66,9 @@ namespace CodeGen {
 // annotation though. For those that don't, the PackOffsets array will contain
 // -1 value instead. These elements must be placed at the end of the layout
 // after all of the elements with specific offset.
-llvm::TargetExtType *HLSLBufferLayoutBuilder::createLayoutType(
-    const RecordType *RT, const llvm::SmallVector<int32_t> *PackOffsets) {
+llvm::TargetExtType *
+HLSLBufferLayoutBuilder::createLayoutType(const RecordType *RT,
+                                          const CGHLSLOffsetInfo &OffsetInfo) {
 
   // check if we already have the layout type for this struct
   if (llvm::TargetExtType *Ty =
@@ -101,14 +102,10 @@ llvm::TargetExtType *HLSLBufferLayoutBuilder::createLayoutType(
     const CXXRecordDecl *RD = RecordDecls.pop_back_val();
 
     for (const auto *FD : RD->fields()) {
-      assert((!PackOffsets || Index < PackOffsets->size()) &&
-             "number of elements in layout struct does not match number of "
-             "packoffset annotations");
-
       // No PackOffset info at all, or have a valid packoffset/register(c#)
       // annotations value -> layout the field.
-      const int PO = PackOffsets ? (*PackOffsets)[Index++] : -1;
-      if (!PackOffsets || PO != -1) {
+      const uint32_t PO = OffsetInfo[Index++];
+      if (PO != CGHLSLOffsetInfo::Unspecified) {
         if (!layoutField(FD, EndOffset, FieldOffset, FieldType, PO))
           return nullptr;
         Layout.push_back(FieldOffset);
@@ -175,7 +172,7 @@ bool HLSLBufferLayoutBuilder::layoutField(const FieldDecl *FD,
                                           unsigned &EndOffset,
                                           unsigned &FieldOffset,
                                           llvm::Type *&FieldType,
-                                          int Packoffset) {
+                                          uint32_t Packoffset) {
 
   // Size of element; for arrays this is a size of a single element in the
   // array. Total array size of calculated as (ArrayCount-1) * ArrayStride +
@@ -201,8 +198,10 @@ bool HLSLBufferLayoutBuilder::layoutField(const FieldDecl *FD,
     // For array of structures, create a new array with a layout type
     // instead of the structure type.
     if (Ty->isStructureOrClassType()) {
+      // TODO: Can we have offsets if we get here from `ConstantBuffer<T>`?
+      CGHLSLOffsetInfo EmptyOffsets;
       llvm::Type *NewTy = cast<llvm::TargetExtType>(
-          createLayoutType(Ty->getAsCanonical<RecordType>()));
+          createLayoutType(Ty->getAsCanonical<RecordType>(), EmptyOffsets));
       if (!NewTy)
         return false;
       assert(isa<llvm::TargetExtType>(NewTy) && "expected target type");
@@ -216,17 +215,21 @@ bool HLSLBufferLayoutBuilder::layoutField(const FieldDecl *FD,
       ElemLayoutTy = CGM.getTypes().ConvertTypeForMem(FieldTy);
     }
     ArrayStride = llvm::alignTo(ElemSize, CBufferRowSizeInBytes);
-    ElemOffset = (Packoffset != -1) ? Packoffset : NextRowOffset;
+    ElemOffset = (Packoffset != CGHLSLOffsetInfo::Unspecified) ? Packoffset
+                                                               : NextRowOffset;
 
   } else if (FieldTy->isStructureOrClassType()) {
     // Create a layout type for the structure
+    // TODO: Can we have offsets if we get here from `ConstantBuffer<T>`?
+    CGHLSLOffsetInfo EmptyOffsets;
     ElemLayoutTy = createLayoutType(
-        cast<RecordType>(FieldTy->getAsCanonical<RecordType>()));
+        cast<RecordType>(FieldTy->getAsCanonical<RecordType>()), EmptyOffsets);
     if (!ElemLayoutTy)
       return false;
     assert(isa<llvm::TargetExtType>(ElemLayoutTy) && "expected target type");
     ElemSize = cast<llvm::TargetExtType>(ElemLayoutTy)->getIntParameter(0);
-    ElemOffset = (Packoffset != -1) ? Packoffset : NextRowOffset;
+    ElemOffset = (Packoffset != CGHLSLOffsetInfo::Unspecified) ? Packoffset
+                                                               : NextRowOffset;
 
   } else {
     // scalar or vector - find element size and alignment
@@ -246,7 +249,7 @@ bool HLSLBufferLayoutBuilder::layoutField(const FieldDecl *FD,
     }
 
     // calculate or get element offset for the vector or scalar
-    if (Packoffset != -1) {
+    if (Packoffset != CGHLSLOffsetInfo::Unspecified) {
       ElemOffset = Packoffset;
     } else {
       ElemOffset = llvm::alignTo(EndOffset, Align);
@@ -269,5 +272,13 @@ bool HLSLBufferLayoutBuilder::layoutField(const FieldDecl *FD,
   return true;
 }
 
+bool HLSLBufferLayoutBuilder::layoutField(const FieldDecl *FD,
+                                          unsigned &EndOffset,
+                                          unsigned &FieldOffset,
+                                          llvm::Type *&FieldType) {
+  return layoutField(FD, EndOffset, FieldOffset, FieldType,
+                     CGHLSLOffsetInfo::Unspecified);
+}
+
 } // namespace CodeGen
 } // namespace clang

clang/lib/CodeGen/HLSLBufferLayoutBuilder.h

@@ -14,6 +14,7 @@ class RecordType;
 class FieldDecl;
 
 namespace CodeGen {
+class CGHLSLOffsetInfo;
 class CodeGenModule;
 
 //===----------------------------------------------------------------------===//
@@ -33,14 +34,15 @@ class HLSLBufferLayoutBuilder {
   // Returns LLVM target extension type with the name LayoutTypeName
   // for given structure type and layout data. The first number in
   // the Layout is the size followed by offsets for each struct element.
-  llvm::TargetExtType *
-  createLayoutType(const RecordType *StructType,
-                   const llvm::SmallVector<int32_t> *Packoffsets = nullptr);
+  llvm::TargetExtType *createLayoutType(const RecordType *StructType,
+                                        const CGHLSLOffsetInfo &OffsetInfo);
 
 private:
   bool layoutField(const clang::FieldDecl *FD, unsigned &EndOffset,
                    unsigned &FieldOffset, llvm::Type *&FieldType,
-                   int Packoffset = -1);
+                   uint32_t Packoffset);
+  bool layoutField(const clang::FieldDecl *FD, unsigned &EndOffset,
+                   unsigned &FieldOffset, llvm::Type *&FieldType);
 };
 
 } // namespace CodeGen

clang/lib/CodeGen/TargetInfo.h

@@ -39,6 +39,7 @@ class ABIInfo;
 class CallArgList;
 class CodeGenFunction;
 class CGBlockInfo;
+class CGHLSLOffsetInfo;
 class SwiftABIInfo;
 
 /// TargetCodeGenInfo - This class organizes various target-specific
@@ -442,9 +443,8 @@ class TargetCodeGenInfo {
   }
 
   /// Return an LLVM type that corresponds to a HLSL type
-  virtual llvm::Type *
-  getHLSLType(CodeGenModule &CGM, const Type *T,
-              const SmallVector<int32_t> *Packoffsets = nullptr) const {
+  virtual llvm::Type *getHLSLType(CodeGenModule &CGM, const Type *T,
+                                  const CGHLSLOffsetInfo &OffsetInfo) const {
     return nullptr;
   }
 

clang/lib/CodeGen/Targets/DirectX.cpp

@@ -29,14 +29,13 @@ class DirectXTargetCodeGenInfo : public TargetCodeGenInfo {
   DirectXTargetCodeGenInfo(CodeGen::CodeGenTypes &CGT)
       : TargetCodeGenInfo(std::make_unique<DefaultABIInfo>(CGT)) {}
 
-  llvm::Type *
-  getHLSLType(CodeGenModule &CGM, const Type *T,
-              const SmallVector<int32_t> *Packoffsets = nullptr) const override;
+  llvm::Type *getHLSLType(CodeGenModule &CGM, const Type *T,
+                          const CGHLSLOffsetInfo &OffsetInfo) const override;
 };
 
 llvm::Type *DirectXTargetCodeGenInfo::getHLSLType(
     CodeGenModule &CGM, const Type *Ty,
-    const SmallVector<int32_t> *Packoffsets) const {
+    const CGHLSLOffsetInfo &OffsetInfo) const {
   auto *ResType = dyn_cast<HLSLAttributedResourceType>(Ty);
   if (!ResType)
     return nullptr;
@@ -78,7 +77,7 @@ llvm::Type *DirectXTargetCodeGenInfo::getHLSLType(
     llvm::Type *BufferLayoutTy =
         HLSLBufferLayoutBuilder(CGM, "dx.Layout")
             .createLayoutType(ContainedTy->castAsCanonical<RecordType>(),
-                              Packoffsets);
+                              OffsetInfo);
     if (!BufferLayoutTy)
       return nullptr;
 

clang/lib/CodeGen/Targets/SPIR.cpp

@@ -53,9 +53,8 @@ class CommonSPIRTargetCodeGenInfo : public TargetCodeGenInfo {
 
   unsigned getDeviceKernelCallingConv() const override;
   llvm::Type *getOpenCLType(CodeGenModule &CGM, const Type *T) const override;
-  llvm::Type *
-  getHLSLType(CodeGenModule &CGM, const Type *Ty,
-              const SmallVector<int32_t> *Packoffsets = nullptr) const override;
+  llvm::Type *getHLSLType(CodeGenModule &CGM, const Type *Ty,
+                          const CGHLSLOffsetInfo &OffsetInfo) const override;
   llvm::Type *getSPIRVImageTypeFromHLSLResource(
       const HLSLAttributedResourceType::Attributes &attributes,
       QualType SampledType, CodeGenModule &CGM) const;
@@ -510,7 +509,7 @@ static llvm::Type *getInlineSpirvType(CodeGenModule &CGM,
 
 llvm::Type *CommonSPIRTargetCodeGenInfo::getHLSLType(
     CodeGenModule &CGM, const Type *Ty,
-    const SmallVector<int32_t> *Packoffsets) const {
+    const CGHLSLOffsetInfo &OffsetInfo) const {
   llvm::LLVMContext &Ctx = CGM.getLLVMContext();
 
   if (auto *SpirvType = dyn_cast<HLSLInlineSpirvType>(Ty))
@@ -559,7 +558,7 @@ llvm::Type *CommonSPIRTargetCodeGenInfo::getHLSLType(
     llvm::Type *BufferLayoutTy =
         HLSLBufferLayoutBuilder(CGM, "spirv.Layout")
             .createLayoutType(ContainedTy->castAsCanonical<RecordType>(),
-                              Packoffsets);
+                              OffsetInfo);
     uint32_t StorageClass = /* Uniform storage class */ 2;
     return llvm::TargetExtType::get(Ctx, "spirv.VulkanBuffer", {BufferLayoutTy},
                                     {StorageClass, false});