Skip to content

[HLSL] Codegen for cbuffer declarations without embedded arrays or structs #119755

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed

[HLSL] Codegen for cbuffer declarations without embedded arrays or structs #119755

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8cebb59
[HLSL] Codegen for simple `cbuffer` blocks without embedded arrays or…
hekota Dec 12, 2024
dee03ca
couple of commas to improve readability
hekota Dec 12, 2024
7274d06
Move target type and cbuffer layout calculations to DirectXTargetInfo
hekota Dec 19, 2024
630fbaf
Merge branch 'main' of https://github.com/llvm/llvm-project into cbuf…
hekota Dec 19, 2024
a150b1e
update tests after merge
hekota Dec 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions clang/include/clang/Basic/Attr.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4711,9 +4711,9 @@ def HLSLPackOffset: HLSLAnnotationAttr {
let Args = [IntArgument<"Subcomponent">, IntArgument<"Component">];
let Documentation = [HLSLPackOffsetDocs];
let AdditionalMembers = [{
unsigned getOffset() {
return subcomponent * 4 + component;
}
unsigned getOffsetInBytes() {
return subcomponent * 16 + component * 4;
}
}];
}

Expand Down
12 changes: 4 additions & 8 deletions clang/lib/CodeGen/CGDeclCXX.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -890,6 +890,10 @@ CodeGenModule::EmitCXXGlobalInitFunc() {
ModuleInits.push_back(Fn);
}

if (getLangOpts().HLSL && getHLSLRuntime().needsResourceBindingInitFn()) {
CXXGlobalInits.push_back(getHLSLRuntime().createResourceBindingInitFn());
}
Comment on lines +893 to +895
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably haven't seen this, but I filed an issue yesterday about how we were connecting initializers in the wrong place (#120636).

This is definitely better. I do wonder if we should be generating one binding function for all bindings (which would require collecting them and generating them late), or if we should be generating an initializer per binding and appending them to the CXXGlobalInits list as we process each binding. While the later produces more functions that need to be inlined, I'm inclined to prefer it because it is simpler and aligns more with a traditional RAII-model for resource initialization.

The issue I filed is probably redundant since you're still reworking all of this.


if (ModuleInits.empty() && CXXGlobalInits.empty() &&
PrioritizedCXXGlobalInits.empty())
return;
Expand Down Expand Up @@ -1131,14 +1135,6 @@ CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn,
if (Decls[i])
EmitRuntimeCall(Decls[i]);

if (getLangOpts().HLSL) {
CGHLSLRuntime &CGHLSL = CGM.getHLSLRuntime();
if (CGHLSL.needsResourceBindingInitFn()) {
llvm::Function *ResInitFn = CGHLSL.createResourceBindingInitFn();
Builder.CreateCall(llvm::FunctionCallee(ResInitFn), {});
}
}

Scope.ForceCleanup();

if (ExitBlock) {
Expand Down
248 changes: 147 additions & 101 deletions clang/lib/CodeGen/CGHLSLRuntime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "CGDebugInfo.h"
#include "CodeGenModule.h"
#include "TargetInfo.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Decl.h"
#include "clang/Basic/TargetOptions.h"
#include "llvm/IR/GlobalVariable.h"
Expand All @@ -29,7 +30,6 @@

using namespace clang;
using namespace CodeGen;
using namespace clang::hlsl;
using namespace llvm;

namespace {
Expand All @@ -54,78 +54,108 @@ void addDxilValVersion(StringRef ValVersionStr, llvm::Module &M) {
auto *DXILValMD = M.getOrInsertNamedMetadata(DXILValKey);
DXILValMD->addOperand(Val);
}

void addDisableOptimizations(llvm::Module &M) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intentional white space change to separate the functions.

StringRef Key = "dx.disable_optimizations";
M.addModuleFlag(llvm::Module::ModFlagBehavior::Override, Key, 1);
}
// cbuffer will be translated into global variable in special address space.
// If translate into C,
// cbuffer A {
// float a;
// float b;
// }
// float foo() { return a + b; }

// Creates resource handle representing the constant buffer.
// For cbuffer declaration:
//
// will be translated into
// cbuffer MyConstants {
// float a;
// }
//
// struct A {
// float a;
// float b;
// } cbuffer_A __attribute__((address_space(4)));
// float foo() { return cbuffer_A.a + cbuffer_A.b; }
// creates a structure type MyConstants and then returns the resource handle
// that would be spelled as:
//
// layoutBuffer will create the struct A type.
// replaceBuffer will replace use of global variable a and b with cbuffer_A.a
// and cbuffer_A.b.
// __hlsl_resource_t [[hlsl::resource_class(CBuffer)]]
// [[contained_type(MyConstants)]]
//
void layoutBuffer(CGHLSLRuntime::Buffer &Buf, const DataLayout &DL) {
if (Buf.Constants.empty())
return;

std::vector<llvm::Type *> EltTys;
for (auto &Const : Buf.Constants) {
GlobalVariable *GV = Const.first;
Const.second = EltTys.size();
llvm::Type *Ty = GV->getValueType();
EltTys.emplace_back(Ty);
static const clang::Type *getBufferHandleType(CGHLSLRuntime::Buffer &Buf) {
HLSLBufferDecl *BD = Buf.Decl;
ASTContext &AST = BD->getASTContext();

// create struct type for the constant buffer; filter out any declarations
// that are not a VarDecls or that are static
CXXRecordDecl *StructDecl = CXXRecordDecl::Create(
BD->getASTContext(), TagDecl::TagKind::Class, BD->getDeclContext(),
BD->getLocation(), BD->getLocation(), BD->getIdentifier());
StructDecl->startDefinition();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This concerns me. We really shouldn't be generating AST types during CodeGen.

We should generate the struct decl during Sema and attach it to the buffer decl so that we can dump and inspect the AST.

for (Decl *it : Buf.Decl->decls()) {
const VarDecl *VD = dyn_cast<VarDecl>(it);
if (!VD || VD->getStorageClass() == SC_Static)
continue;
auto *Field = FieldDecl::Create(
AST, StructDecl, VD->getLocation(), VD->getLocation(),
VD->getIdentifier(), VD->getType(), VD->getTypeSourceInfo(), nullptr,
false, InClassInitStyle::ICIS_NoInit);
Field->setAccess(AccessSpecifier::AS_private);
StructDecl->addDecl(Field);
}
Buf.LayoutStruct = llvm::StructType::get(EltTys[0]->getContext(), EltTys);
StructDecl->completeDefinition();
assert(!StructDecl->fields().empty() && "empty cbuffer should not get here");

// create the resource handle type
HLSLAttributedResourceType::Attributes ResAttrs(dxil::ResourceClass::CBuffer,
false, false);
QualType ContainedTy = QualType(StructDecl->getTypeForDecl(), 0);
return AST
.getHLSLAttributedResourceType(AST.HLSLResourceTy, ContainedTy, ResAttrs)
.getTypePtr();
}

GlobalVariable *replaceBuffer(CGHLSLRuntime::Buffer &Buf) {
// Create global variable for CB.
GlobalVariable *CBGV = new GlobalVariable(
Buf.LayoutStruct, /*isConstant*/ true,
GlobalValue::LinkageTypes::ExternalLinkage, nullptr,
llvm::formatv("{0}{1}", Buf.Name, Buf.IsCBuffer ? ".cb." : ".tb."),
GlobalValue::NotThreadLocal);

IRBuilder<> B(CBGV->getContext());
Value *ZeroIdx = B.getInt32(0);
// Replace Const use with CB use.
for (auto &[GV, Offset] : Buf.Constants) {
Value *GEP =
B.CreateGEP(Buf.LayoutStruct, CBGV, {ZeroIdx, B.getInt32(Offset)});

assert(Buf.LayoutStruct->getElementType(Offset) == GV->getValueType() &&
"constant type mismatch");

// Replace.
GV->replaceAllUsesWith(GEP);
// Erase GV.
GV->removeDeadConstantUsers();
GV->eraseFromParent();
// Replaces all uses of the temporary constant buffer global variables with
// buffer access intrinsic resource.getpointer and GEP.
static void replaceBufferGlobals(CodeGenModule &CGM,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a big no. This is falling into the mess that was CGHLSLMSFinishCodeGen in DXC.

We should not be manipulating IR in Clang CodeGen. We should be able to inspect and dump the post-codegen IR, and test every IR transformation independently.

CGHLSLRuntime::Buffer &Buf) {
assert(Buf.IsCBuffer && "tbuffer codegen is not yet supported");

GlobalVariable *BufGV = Buf.GlobalVar;
llvm::Type *TargetTy = BufGV->getValueType();
llvm::Type *BufStructTy = cast<TargetExtType>(TargetTy)->getTypeParameter(0);
unsigned Index = 0;
for (auto ConstIt = Buf.Constants.begin(); ConstIt != Buf.Constants.end();
++ConstIt, ++Index) {
GlobalVariable *ConstGV = *ConstIt;

// TODO: Map to an hlsl_device address space.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// TODO: Map to an hlsl_device address space.
// TODO: Map to an hlsl_device address space (#109877).

llvm::Type *RetTy = ConstGV->getType();

// Replace all uses of GV with CBuffer access
while (ConstGV->use_begin() != ConstGV->use_end()) {
Use &U = *ConstGV->use_begin();
if (Instruction *UserInstr = dyn_cast<Instruction>(U.getUser())) {
IRBuilder<> Builder(UserInstr);
Value *Zero = Builder.getInt32(0);
Value *Handle = Builder.CreateLoad(TargetTy, BufGV);
Value *ResGetPointer =
Builder.CreateIntrinsic(RetTy, Intrinsic::dx_resource_getpointer,
ArrayRef<llvm::Value *>{Handle, Zero});
Value *GEP = Builder.CreateGEP(BufStructTy, ResGetPointer,
{Zero, Builder.getInt32(Index)},
ConstGV->getName());
U.set(GEP);
} else {
llvm_unreachable("unexpected use of constant value");
}
}
ConstGV->removeDeadConstantUsers();
ConstGV->eraseFromParent();
}
return CBGV;
}

} // namespace

llvm::Type *CGHLSLRuntime::convertHLSLSpecificType(const Type *T) {
llvm::Type *
CGHLSLRuntime::convertHLSLSpecificType(const Type *T,
const HLSLBufferDecl *BufferDecl) {
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))
if (llvm::Type *TargetTy =
CGM.getTargetCodeGenInfo().getHLSLType(CGM, T, BufferDecl))
return TargetTy;

llvm_unreachable("Generic handling of HLSL types is not supported.");
Expand All @@ -143,20 +173,20 @@ void CGHLSLRuntime::addConstant(VarDecl *D, Buffer &CB) {
return;
}

assert(!D->getType()->isArrayType() && !D->getType()->isStructureType() &&
"codegen for arrays and structs in cbuffer is not yet supported");

auto *GV = cast<GlobalVariable>(CGM.GetAddrOfGlobalVar(D));
// Add debug info for constVal.
if (CGDebugInfo *DI = CGM.getModuleDebugInfo())
if (CGM.getCodeGenOpts().getDebugInfo() >=
codegenoptions::DebugInfoKind::LimitedDebugInfo)
DI->EmitGlobalVariable(cast<GlobalVariable>(GV), D);

// FIXME: support packoffset.
// See https://github.com/llvm/llvm-project/issues/57914.
uint32_t Offset = 0;
bool HasUserOffset = false;
CB.Constants.emplace_back(GV);

unsigned LowerBound = HasUserOffset ? Offset : UINT_MAX;
CB.Constants.emplace_back(std::make_pair(GV, LowerBound));
if (HLSLPackOffsetAttr *PO = D->getAttr<HLSLPackOffsetAttr>())
CB.HasPackoffset = true;
}

void CGHLSLRuntime::addBufferDecls(const DeclContext *DC, Buffer &CB) {
Expand All @@ -173,9 +203,35 @@ void CGHLSLRuntime::addBufferDecls(const DeclContext *DC, Buffer &CB) {
}
}

void CGHLSLRuntime::addBuffer(const HLSLBufferDecl *D) {
Buffers.emplace_back(Buffer(D));
addBufferDecls(D, Buffers.back());
// Creates temporary global variables for all declarations within the constant
// buffer context, creates a global variable for the constant buffer and adds
// it to the module.
// All uses of the temporary constant globals will be replaced with buffer
// access intrinsic resource.getpointer in CGHLSLRuntime::finishCodeGen.
// Later on in DXILResourceAccess pass these will be transtaled
// to dx.op.cbufferLoadLegacy instructions.
void CGHLSLRuntime::addBuffer(HLSLBufferDecl *D) {
llvm::Module &M = CGM.getModule();

assert(D->isCBuffer() && "tbuffer codegen is not supported yet");

Buffer &Buf = Buffers.emplace_back(D);
addBufferDecls(D, Buf);
if (Buf.Constants.empty()) {
// empty constant buffer - do not add to globals
Buffers.pop_back();
return;
}
// Create global variable for CB.
llvm::Type *TargetTy = convertHLSLSpecificType(
getBufferHandleType(Buf), Buf.HasPackoffset ? Buf.Decl : nullptr);
Buf.GlobalVar = new GlobalVariable(
TargetTy, /*isConstant*/ true, GlobalValue::LinkageTypes::ExternalLinkage,
nullptr, llvm::formatv("{0}{1}", Buf.Name, Buf.IsCBuffer ? ".cb" : ".tb"),
GlobalValue::NotThreadLocal);

M.insertGlobalVariable(Buf.GlobalVar);
ResourcesToBind.emplace_back(Buf.Decl, Buf.GlobalVar);
}

void CGHLSLRuntime::finishCodeGen() {
Expand All @@ -189,26 +245,14 @@ void CGHLSLRuntime::finishCodeGen() {
if (CGM.getCodeGenOpts().OptimizationLevel == 0)
addDisableOptimizations(M);

const DataLayout &DL = M.getDataLayout();

for (auto &Buf : Buffers) {
layoutBuffer(Buf, DL);
GlobalVariable *GV = replaceBuffer(Buf);
M.insertGlobalVariable(GV);
llvm::hlsl::ResourceClass RC = Buf.IsCBuffer
? llvm::hlsl::ResourceClass::CBuffer
: llvm::hlsl::ResourceClass::SRV;
llvm::hlsl::ResourceKind RK = Buf.IsCBuffer
? llvm::hlsl::ResourceKind::CBuffer
: llvm::hlsl::ResourceKind::TBuffer;
addBufferResourceAnnotation(GV, RC, RK, /*IsROV=*/false,
llvm::hlsl::ElementType::Invalid, Buf.Binding);
replaceBufferGlobals(CGM, Buf);
}
}

CGHLSLRuntime::Buffer::Buffer(const HLSLBufferDecl *D)
: Name(D->getName()), IsCBuffer(D->isCBuffer()),
Binding(D->getAttr<HLSLResourceBindingAttr>()) {}
CGHLSLRuntime::Buffer::Buffer(HLSLBufferDecl *D)
: Name(D->getName()), IsCBuffer(D->isCBuffer()), HasPackoffset(false),
Decl(D), GlobalVar(nullptr) {}

void CGHLSLRuntime::addBufferResourceAnnotation(llvm::GlobalVariable *GV,
llvm::hlsl::ResourceClass RC,
Expand Down Expand Up @@ -237,7 +281,7 @@ void CGHLSLRuntime::addBufferResourceAnnotation(llvm::GlobalVariable *GV,
"ResourceMD must have been set by the switch above.");

llvm::hlsl::FrontendResource Res(
GV, RK, ET, IsROV, Binding.Reg.value_or(UINT_MAX), Binding.Space);
GV, RK, ET, IsROV, Binding.Slot.value_or(UINT_MAX), Binding.Space);
ResourceMD->addOperand(Res.getMetadata());
}

Expand Down Expand Up @@ -328,12 +372,8 @@ void CGHLSLRuntime::annotateHLSLResource(const VarDecl *D, GlobalVariable *GV) {
CGHLSLRuntime::BufferResBinding::BufferResBinding(
HLSLResourceBindingAttr *Binding) {
if (Binding) {
llvm::APInt RegInt(64, 0);
Binding->getSlot().substr(1).getAsInteger(10, RegInt);
Reg = RegInt.getLimitedValue();
llvm::APInt SpaceInt(64, 0);
Binding->getSpace().substr(5).getAsInteger(10, SpaceInt);
Space = SpaceInt.getLimitedValue();
Slot = Binding->getSlotNumber();
Space = Binding->getSpaceNumber();
} else {
Space = 0;
}
Expand Down Expand Up @@ -576,24 +616,30 @@ llvm::Function *CGHLSLRuntime::createResourceBindingInitFn() {
const DataLayout &DL = CGM.getModule().getDataLayout();
Builder.SetInsertPoint(EntryBB);

for (const auto &[VD, GV] : ResourcesToBind) {
for (Attr *A : VD->getAttrs()) {
for (const auto &[Decl, GV] : ResourcesToBind) {
for (Attr *A : Decl->getAttrs()) {
HLSLResourceBindingAttr *RBA = dyn_cast<HLSLResourceBindingAttr>(A);
if (!RBA)
continue;

const HLSLAttributedResourceType *AttrResType =
HLSLAttributedResourceType::findHandleTypeOnResource(
VD->getType().getTypePtr());

// FIXME: Only simple declarations of resources are supported for now.
// Arrays of resources or resources in user defined classes are
// not implemented yet.
assert(AttrResType != nullptr &&
"Resource class must have a handle of HLSLAttributedResourceType");

llvm::Type *TargetTy =
CGM.getTargetCodeGenInfo().getHLSLType(CGM, AttrResType);
llvm::Type *TargetTy = nullptr;
if (const VarDecl *VD = dyn_cast<VarDecl>(Decl)) {
const HLSLAttributedResourceType *AttrResType =
HLSLAttributedResourceType::findHandleTypeOnResource(
VD->getType().getTypePtr());

// FIXME: Only simple declarations of resources are supported for now.
// Arrays of resources or resources in user defined classes are
// not implemented yet.
assert(
AttrResType != nullptr &&
"Resource class must have a handle of HLSLAttributedResourceType");

TargetTy = CGM.getTargetCodeGenInfo().getHLSLType(CGM, AttrResType);
} else {
assert(isa<HLSLBufferDecl>(Decl));
TargetTy = GV->getValueType();
}
assert(TargetTy != nullptr &&
"Failed to convert resource handle to target type");

Expand All @@ -608,7 +654,7 @@ llvm::Function *CGHLSLRuntime::createResourceBindingInitFn() {

llvm::Value *CreateHandle = Builder.CreateIntrinsic(
/*ReturnType=*/TargetTy, getCreateHandleFromBindingIntrinsic(), Args,
nullptr, Twine(VD->getName()).concat("_h"));
nullptr, Twine(Decl->getName()).concat("_h"));

llvm::Value *HandleRef =
Builder.CreateStructGEP(GV->getValueType(), GV, 0);
Expand Down
Loading
Loading