Skip to content
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

[HLSL] Introduce address space hlsl_constant(2) for constant buffer declarations #123411

Merged
merged 4 commits into from
Jan 25, 2025
Merged

[HLSL] Introduce address space hlsl_constant(2) for constant buffer declarations #123411

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ac04912
[HLSL] Add address space `hlsl_constant(2)` for constant buffer decla…
hekota Jan 17, 2025
3d813ec
code review feedback
hekota Jan 21, 2025
581d2b3
map hlsl_constant to CONSTANT_ADDRESS for AMDGPU
hekota Jan 22, 2025
404de38
Update tests after rebase
hekota Jan 24, 2025
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
1 change: 1 addition & 0 deletions clang/include/clang/Basic/AddressSpaces.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ enum class LangAS : unsigned {

// HLSL specific address spaces.
hlsl_groupshared,
hlsl_constant,

// Wasm specific address spaces.
wasm_funcref,
Expand Down
6 changes: 4 additions & 2 deletions clang/lib/AST/TypePrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2552,10 +2552,12 @@ std::string Qualifiers::getAddrSpaceAsString(LangAS AS) {
return "__uptr __ptr32";
case LangAS::ptr64:
return "__ptr64";
case LangAS::wasm_funcref:
return "__funcref";
case LangAS::hlsl_groupshared:
return "groupshared";
case LangAS::hlsl_constant:
return "hlsl_constant";
case LangAS::wasm_funcref:
return "__funcref";
default:
return std::to_string(toTargetAddressSpace(AS));
}
Expand Down
1 change: 1 addition & 0 deletions clang/lib/Basic/Targets/AArch64.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ static const unsigned ARM64AddrSpaceMap[] = {
static_cast<unsigned>(AArch64AddrSpace::ptr32_uptr),
static_cast<unsigned>(AArch64AddrSpace::ptr64),
0, // hlsl_groupshared
0, // hlsl_constant
// Wasm address space values for this target are dummy values,
// as it is only enabled for Wasm targets.
20, // wasm_funcref
Expand Down
21 changes: 11 additions & 10 deletions clang/lib/Basic/Targets/AMDGPU.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ const LangASMap AMDGPUTargetInfo::AMDGPUDefIsGenMap = {
llvm::AMDGPUAS::FLAT_ADDRESS, // ptr32_uptr
llvm::AMDGPUAS::FLAT_ADDRESS, // ptr64
llvm::AMDGPUAS::FLAT_ADDRESS, // hlsl_groupshared
llvm::AMDGPUAS::CONSTANT_ADDRESS, // hlsl_constant
};

const LangASMap AMDGPUTargetInfo::AMDGPUDefIsPrivMap = {
Expand All @@ -74,16 +75,16 @@ const LangASMap AMDGPUTargetInfo::AMDGPUDefIsPrivMap = {
llvm::AMDGPUAS::CONSTANT_ADDRESS, // cuda_constant
llvm::AMDGPUAS::LOCAL_ADDRESS, // cuda_shared
// SYCL address space values for this map are dummy
llvm::AMDGPUAS::FLAT_ADDRESS, // sycl_global
llvm::AMDGPUAS::FLAT_ADDRESS, // sycl_global_device
llvm::AMDGPUAS::FLAT_ADDRESS, // sycl_global_host
llvm::AMDGPUAS::FLAT_ADDRESS, // sycl_local
llvm::AMDGPUAS::FLAT_ADDRESS, // sycl_private
llvm::AMDGPUAS::FLAT_ADDRESS, // ptr32_sptr
llvm::AMDGPUAS::FLAT_ADDRESS, // ptr32_uptr
llvm::AMDGPUAS::FLAT_ADDRESS, // ptr64
llvm::AMDGPUAS::FLAT_ADDRESS, // hlsl_groupshared

llvm::AMDGPUAS::FLAT_ADDRESS, // sycl_global
llvm::AMDGPUAS::FLAT_ADDRESS, // sycl_global_device
llvm::AMDGPUAS::FLAT_ADDRESS, // sycl_global_host
llvm::AMDGPUAS::FLAT_ADDRESS, // sycl_local
llvm::AMDGPUAS::FLAT_ADDRESS, // sycl_private
llvm::AMDGPUAS::FLAT_ADDRESS, // ptr32_sptr
llvm::AMDGPUAS::FLAT_ADDRESS, // ptr32_uptr
llvm::AMDGPUAS::FLAT_ADDRESS, // ptr64
llvm::AMDGPUAS::FLAT_ADDRESS, // hlsl_groupshared
llvm::AMDGPUAS::CONSTANT_ADDRESS, // hlsl_constant
};
} // namespace targets
} // namespace clang
Expand Down
1 change: 1 addition & 0 deletions clang/lib/Basic/Targets/DirectX.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ static const unsigned DirectXAddrSpaceMap[] = {
0, // ptr32_uptr
0, // ptr64
3, // hlsl_groupshared
2, // hlsl_constant
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm surprised we neither use the LangASMap type for this array, nor static_assert that the size is the same, to prevent these from getting out-of-sync. Does the assignment of this array pointer to LangASMap * cause a compilation failure which can be used to enforce map updates when address spaces are added?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, not updating all of the maps produces a compile error.

// Wasm address space values for this target are dummy values,
// as it is only enabled for Wasm targets.
20, // wasm_funcref
Expand Down
1 change: 1 addition & 0 deletions clang/lib/Basic/Targets/NVPTX.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ static const unsigned NVPTXAddrSpaceMap[] = {
0, // ptr32_uptr
0, // ptr64
0, // hlsl_groupshared
0, // hlsl_constant
// Wasm address space values for this target are dummy values,
// as it is only enabled for Wasm targets.
20, // wasm_funcref
Expand Down
2 changes: 2 additions & 0 deletions clang/lib/Basic/Targets/SPIR.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ static const unsigned SPIRDefIsPrivMap[] = {
0, // ptr32_uptr
0, // ptr64
0, // hlsl_groupshared
2, // hlsl_constant
// Wasm address space values for this target are dummy values,
// as it is only enabled for Wasm targets.
20, // wasm_funcref
Expand Down Expand Up @@ -80,6 +81,7 @@ static const unsigned SPIRDefIsGenMap[] = {
0, // ptr32_uptr
0, // ptr64
0, // hlsl_groupshared
0, // hlsl_constant
// Wasm address space values for this target are dummy values,
// as it is only enabled for Wasm targets.
20, // wasm_funcref
Expand Down
1 change: 1 addition & 0 deletions clang/lib/Basic/Targets/SystemZ.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ static const unsigned ZOSAddressMap[] = {
1, // ptr32_uptr
0, // ptr64
0, // hlsl_groupshared
0, // hlsl_constant
0 // wasm_funcref
};

Expand Down
1 change: 1 addition & 0 deletions clang/lib/Basic/Targets/TCE.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ static const unsigned TCEOpenCLAddrSpaceMap[] = {
0, // ptr32_uptr
0, // ptr64
0, // hlsl_groupshared
0, // hlsl_constant
// Wasm address space values for this target are dummy values,
// as it is only enabled for Wasm targets.
20, // wasm_funcref
Expand Down
1 change: 1 addition & 0 deletions clang/lib/Basic/Targets/WebAssembly.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ static const unsigned WebAssemblyAddrSpaceMap[] = {
0, // ptr32_uptr
0, // ptr64
0, // hlsl_groupshared
0, // hlsl_constant
20, // wasm_funcref
};

Expand Down
1 change: 1 addition & 0 deletions clang/lib/Basic/Targets/X86.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ static const unsigned X86AddrSpaceMap[] = {
271, // ptr32_uptr
272, // ptr64
0, // hlsl_groupshared
0, // hlsl_constant
// Wasm address space values for this target are dummy values,
// as it is only enabled for Wasm targets.
20, // wasm_funcref
Expand Down
17 changes: 1 addition & 16 deletions clang/lib/CodeGen/CGHLSLRuntime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,22 +100,6 @@ GlobalVariable *replaceBuffer(CGHLSLRuntime::Buffer &Buf) {
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();
}
return CBGV;
}

Expand Down Expand Up @@ -144,6 +128,7 @@ void CGHLSLRuntime::addConstant(VarDecl *D, Buffer &CB) {
}

auto *GV = cast<GlobalVariable>(CGM.GetAddrOfGlobalVar(D));
GV->setExternallyInitialized(true);
// Add debug info for constVal.
if (CGDebugInfo *DI = CGM.getModuleDebugInfo())
if (CGM.getCodeGenOpts().getDebugInfo() >=
Expand Down
13 changes: 10 additions & 3 deletions clang/lib/Sema/SemaHLSL.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "clang/Basic/IdentifierTable.h"
#include "clang/Basic/LLVM.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/Specifiers.h"
#include "clang/Basic/TargetInfo.h"
#include "clang/Sema/Initialization.h"
#include "clang/Sema/ParsedAttr.h"
Expand Down Expand Up @@ -475,14 +476,20 @@ void createHostLayoutStructForBuffer(Sema &S, HLSLBufferDecl *BufDecl) {
LS->setImplicit(true);
LS->startDefinition();

for (const Decl *D : BufDecl->decls()) {
const VarDecl *VD = dyn_cast<VarDecl>(D);
for (Decl *D : BufDecl->decls()) {
VarDecl *VD = dyn_cast<VarDecl>(D);
if (!VD || VD->getStorageClass() == SC_Static)
continue;
const Type *Ty = VD->getType()->getUnqualifiedDesugaredType();
if (FieldDecl *FD =
createFieldForHostLayoutStruct(S, Ty, VD->getIdentifier(), LS))
createFieldForHostLayoutStruct(S, Ty, VD->getIdentifier(), LS)) {
// add the field decl to the layout struct
LS->addDecl(FD);
// update address space of the original decl to hlsl_constant
QualType NewTy =
AST.getAddrSpaceQualType(VD->getType(), LangAS::hlsl_constant);
VD->setType(NewTy);
}
}
LS->completeDefinition();
BufDecl->addDecl(LS);
Expand Down
62 changes: 0 additions & 62 deletions clang/test/AST/HLSL/ast-dump-comment-cbuffer-tbuffer.hlsl
View file
Open in desktop

This file was deleted.

32 changes: 32 additions & 0 deletions clang/test/AST/HLSL/ast-dump-comment-cbuffer.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// RUN: %clang_cc1 -Wdocumentation -ast-dump=json -x hlsl -triple dxil-pc-shadermodel6.3-library %s | FileCheck %s --check-prefix=JSON
// RUN: %clang_cc1 -Wdocumentation -ast-dump -x hlsl -triple dxil-pc-shadermodel6.3-library %s | FileCheck %s --check-prefix=AST

// JSON:"kind": "HLSLBufferDecl",
// JSON:"name": "A",
// JSON-NEXT:"bufferKind": "cbuffer",
// JSON:"kind": "TextComment",
// JSON:"text": " CBuffer decl."

/// CBuffer decl.
cbuffer A {
// JSON: "kind": "VarDecl",
// JSON: "name": "a",
// JSON: "qualType": "hlsl_constant float"
float a;
// JSON: "kind": "VarDecl",
// JSON: "name": "b",
// JSON: "qualType": "hlsl_constant int"
int b;
}

// AST: HLSLBufferDecl {{.*}} line:11:9 cbuffer A
// AST-NEXT: HLSLResourceClassAttr {{.*}} Implicit CBuffer
// AST-NEXT: HLSLResourceAttr {{.*}} Implicit CBuffer
// AST-NEXT: FullComment
// AST-NEXT: ParagraphComment
// AST-NEXT: TextComment {{.*}} Text=" CBuffer decl."
// AST-NEXT: VarDecl {{.*}} a 'hlsl_constant float'
// AST-NEXT: VarDecl {{.*}} b 'hlsl_constant int'
// AST-NEXT: CXXRecordDecl {{.*}} implicit class __layout_A definition
// AST: FieldDecl {{.*}} a 'float'
// AST-NEXT: FieldDecl {{.*}} b 'int'
24 changes: 12 additions & 12 deletions clang/test/AST/HLSL/cbuffer.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ struct TwoFloats {
// CHECK: HLSLResourceClassAttr {{.*}} Implicit CBuffer
// CHECK: HLSLResourceAttr {{.*}} Implicit CBuffer
cbuffer CB {
// CHECK: VarDecl {{.*}} col:9 used a1 'float'
// CHECK: VarDecl {{.*}} col:9 used a1 'hlsl_constant float'
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this also impact type names in diagnostics?

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably, the address space is attached to QualType.

float a1;
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_CB definition
// CHECK: FieldDecl {{.*}} a1 'float'
Expand All @@ -60,15 +60,15 @@ _Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(OneFloat, __layout
// CHECK: HLSLResourceClassAttr {{.*}} Implicit CBuffer
// CHECK: HLSLResourceAttr {{.*}} Implicit CBuffer
cbuffer CB {
// CHECK: VarDecl {{.*}} col:9 used a2 'float'
// CHECK: VarDecl {{.*}} col:9 used a2 'hlsl_constant float'
float a2;
// CHECK: VarDecl {{.*}} col:19 b2 'RWBuffer<float>':'hlsl::RWBuffer<float>'
RWBuffer<float> b2;
// CHECK: VarDecl {{.*}} col:15 c2 'EmptyStruct'
EmptyStruct c2;
// CHECK: VarDecl {{.*}} col:9 d2 'float[0]'
float d2[0];
// CHECK: VarDecl {{.*}} col:9 e2 'float'
// CHECK: VarDecl {{.*}} col:9 e2 'hlsl_constant float'
float e2;
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_CB_1 definition
// CHECK: FieldDecl {{.*}} a2 'float'
Expand All @@ -81,11 +81,11 @@ _Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(TwoFloats, __layou
// CHECK: HLSLResourceClassAttr {{.*}} Implicit CBuffer
// CHECK: HLSLResourceAttr {{.*}} Implicit CBuffer
cbuffer CB {
// CHECK: VarDecl {{.*}} col:5 used s1 'A'
// CHECK: VarDecl {{.*}} col:5 used s1 'hlsl_constant A'
A s1;
// CHECK: VarDecl {{.*}} col:5 s2 'B'
// CHECK: VarDecl {{.*}} col:5 s2 'hlsl_constant B'
B s2;
// CHECK: VarDecl {{.*}} col:12 s3 'CTypedef':'C
// CHECK: VarDecl {{.*}} col:12 s3 'CTypedef':'C'
CTypedef s3;
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_CB_2 definition
// CHECK: FieldDecl {{.*}} s1 'A'
Expand All @@ -102,7 +102,7 @@ _Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(TwoFloats, __layou
// CHECK: HLSLResourceClassAttr {{.*}} Implicit CBuffer
// CHECK: HLSLResourceAttr {{.*}} Implicit CBuffer
cbuffer CB {
// CHECK: VarDecl {{.*}} s4 'D'
// CHECK: VarDecl {{.*}} s4 'hlsl_constant D'
D s4;
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_CB_3 definition
// CHECK: FieldDecl {{.*}} s4 '__layout_D'
Expand All @@ -120,9 +120,9 @@ _Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(TwoFloats, __layou
// CHECK: HLSLResourceClassAttr {{.*}} Implicit CBuffer
// CHECK: HLSLResourceAttr {{.*}} Implicit CBuffer
cbuffer CB {
// CHECK: VarDecl {{.*}} s5 'E'
// CHECK: VarDecl {{.*}} s5 'hlsl_constant E'
E s5;
// CHECK: VarDecl {{.*}} s6 'BTypedef':'B'
// CHECK: VarDecl {{.*}} s6 'hlsl_constant BTypedef':'hlsl_constant B'
BTypedef s6;
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_CB_4 definition
// CHECK: FieldDecl {{.*}} s5 '__layout_E'
Expand Down Expand Up @@ -158,7 +158,7 @@ cbuffer CB {
// CHECK: HLSLResourceClassAttr {{.*}} Implicit CBuffer
// CHECK: HLSLResourceAttr {{.*}} Implicit CBuffer
cbuffer CB {
// CHECK: VarDecl {{.*}} s8 'F'
// CHECK: VarDecl {{.*}} s8 'hlsl_constant F'
F s8;
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_CB_6 definition
// CHECK: FieldDecl {{.*}} s8 '__layout_F'
Expand All @@ -182,15 +182,15 @@ cbuffer CB {
// CHECK: FieldDecl {{.*}} f 'RWBuffer<float>':'hlsl::RWBuffer<float>'
RWBuffer<float> f;
} s9;
// CHECK: VarDecl {{.*}} s9 'struct (unnamed struct at {{.*}}cbuffer.hlsl:177:3
// CHECK: VarDecl {{.*}} s9 'hlsl_constant struct (unnamed struct at {{.*}}cbuffer.hlsl:177:3
// CHECK: CXXRecordDecl {{.*}} struct definition
struct {
// CHECK: FieldDecl {{.*}} g 'float'
float g;
// CHECK: FieldDecl {{.*}} f 'RWBuffer<float>':'hlsl::RWBuffer<float>'
RWBuffer<float> f;
} s10;
// CHECK: VarDecl {{.*}} s10 'struct (unnamed struct at {{.*}}cbuffer.hlsl:187:3
// CHECK: VarDecl {{.*}} s10 'hlsl_constant struct (unnamed struct at {{.*}}cbuffer.hlsl:187:3
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_anon definition
// CHECK: FieldDecl {{.*}} e 'float'
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_anon_1 definition
Expand Down
Loading
Loading