[HLSL] Wrap offset info into a dedicated type. NFC #167396
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang-codegen @llvm/pr-subscribers-backend-directx Author: Justin Bogner (bogner) ChangesRather 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. Full diff: https://github.com/llvm/llvm-project/pull/167396.diff 7 Files Affected:
diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index e392a12044a39..4bdba9b3da502 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/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),
diff --git a/clang/lib/CodeGen/CGHLSLRuntime.h b/clang/lib/CodeGen/CGHLSLRuntime.h
index 9d31714ab8606..488a322ca7569 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.h
+++ b/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();
diff --git a/clang/lib/CodeGen/HLSLBufferLayoutBuilder.cpp b/clang/lib/CodeGen/HLSLBufferLayoutBuilder.cpp
index 838903cdcd1ee..63e5452961028 100644
--- a/clang/lib/CodeGen/HLSLBufferLayoutBuilder.cpp
+++ b/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
diff --git a/clang/lib/CodeGen/HLSLBufferLayoutBuilder.h b/clang/lib/CodeGen/HLSLBufferLayoutBuilder.h
index 61240b280cfcb..ec0ad2fd84223 100644
--- a/clang/lib/CodeGen/HLSLBufferLayoutBuilder.h
+++ b/clang/lib/CodeGen/HLSLBufferLayoutBuilder.h
@@ -14,6 +14,7 @@ class RecordType;
class FieldDecl;
namespace CodeGen {
+class CGHLSLOffsetInfo;
class CodeGenModule;
//===----------------------------------------------------------------------===//
@@ -35,12 +36,14 @@ class HLSLBufferLayoutBuilder {
// 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);
+ 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
diff --git a/clang/lib/CodeGen/TargetInfo.h b/clang/lib/CodeGen/TargetInfo.h
index f63e900669d97..383f52f298d2e 100644
--- a/clang/lib/CodeGen/TargetInfo.h
+++ b/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;
}
diff --git a/clang/lib/CodeGen/Targets/DirectX.cpp b/clang/lib/CodeGen/Targets/DirectX.cpp
index b4cebb9a32aca..f30b30284cb12 100644
--- a/clang/lib/CodeGen/Targets/DirectX.cpp
+++ b/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;
diff --git a/clang/lib/CodeGen/Targets/SPIR.cpp b/clang/lib/CodeGen/Targets/SPIR.cpp
index abd049aca0ed7..87c2ebb6a3a89 100644
--- a/clang/lib/CodeGen/Targets/SPIR.cpp
+++ b/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});
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
llvm-beanz
approved these changes
Nov 11, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.