https://github.com/hekota updated
https://github.com/llvm/llvm-project/pull/114148
>From 8c76f28c8a0ba3d087361141366968071fa3af6e Mon Sep 17 00:00:00 2001
From: Helena Kotas <heko...@microsoft.com>
Date: Tue, 29 Oct 2024 16:02:26 -0700
Subject: [PATCH 1/5] [HLSL] Add Increment/DecrementCounter methods to
structured buffers
Introduces `__builtin_hlsl_buffer_update_counter` clang buildin that is used to
implement IncrementCounter and DecrementCounter methods on RWStructuredBuffer
and RasterizerOrderedStructuredBuffer. The builtin is translated to LLVM
intrisics llvm.dx.bufferUpdateCounter/llvm.spv.bufferUpdateCounter.
Introduces `BuiltinTypeMethodBuilder` helper in `HLSLExternalSemaSource` that
allows adding methods to builtin types
using the builder pattern like this:
BuiltinTypeMethodBuilder(Sema, RecordBuilder, "MethodName", ReturnType)
.addParam("param_name", Type, InOutModifier)
.callBuiltin("buildin_name", { BuiltinParams })
.finalizeMethod();
Fixes #113513
---
clang/include/clang/Basic/Builtins.td | 7 +-
.../clang/Basic/DiagnosticSemaKinds.td | 4 +
clang/lib/CodeGen/CGBuiltin.cpp | 8 +
clang/lib/CodeGen/CGHLSLRuntime.h | 1 +
clang/lib/Sema/HLSLExternalSemaSource.cpp | 278 ++++++++++++++++--
clang/lib/Sema/SemaExpr.cpp | 4 +
clang/lib/Sema/SemaHLSL.cpp | 41 +++
.../StructuredBuffers-methods-lib.hlsl | 25 ++
.../StructuredBuffers-methods-ps.hlsl | 29 ++
.../buffer_update_counter-errors.hlsl | 22 ++
llvm/include/llvm/IR/IntrinsicsDirectX.td | 3 +
llvm/include/llvm/IR/IntrinsicsSPIRV.td | 3 +
12 files changed, 393 insertions(+), 32 deletions(-)
create mode 100644
clang/test/CodeGenHLSL/builtins/StructuredBuffers-methods-lib.hlsl
create mode 100644
clang/test/CodeGenHLSL/builtins/StructuredBuffers-methods-ps.hlsl
create mode 100644
clang/test/SemaHLSL/BuiltIns/buffer_update_counter-errors.hlsl
diff --git a/clang/include/clang/Basic/Builtins.td
b/clang/include/clang/Basic/Builtins.td
index 90475a361bb8f8..72bc2d5e7df23e 100644
--- a/clang/include/clang/Basic/Builtins.td
+++ b/clang/include/clang/Basic/Builtins.td
@@ -4846,7 +4846,6 @@ def HLSLSaturate : LangBuiltin<"HLSL_LANG"> {
let Prototype = "void(...)";
}
-
def HLSLSelect : LangBuiltin<"HLSL_LANG"> {
let Spellings = ["__builtin_hlsl_select"];
let Attributes = [NoThrow, Const];
@@ -4871,6 +4870,12 @@ def HLSLRadians : LangBuiltin<"HLSL_LANG"> {
let Prototype = "void(...)";
}
+def HLSLBufferUpdateCounter : LangBuiltin<"HLSL_LANG"> {
+ let Spellings = ["__builtin_hlsl_buffer_update_counter"];
+ let Attributes = [NoThrow, Const];
+ let Prototype = "uint32_t(...)";
+}
+
// Builtins for XRay.
def XRayCustomEvent : Builtin {
let Spellings = ["__xray_customevent"];
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 8e4718008ece72..2aea6bb657578a 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7256,6 +7256,8 @@ def err_typecheck_illegal_increment_decrement : Error<
"cannot %select{decrement|increment}1 value of type %0">;
def err_typecheck_expect_int : Error<
"used type %0 where integer is required">;
+def err_typecheck_expect_hlsl_resource : Error<
+ "used type %0 where __hlsl_resource_t is required">;
def err_typecheck_arithmetic_incomplete_or_sizeless_type : Error<
"arithmetic on a pointer to %select{an incomplete|sizeless}0 type %1">;
def err_typecheck_pointer_arith_function_type : Error<
@@ -12485,6 +12487,8 @@ def warn_attr_min_eq_max: Warning<
def err_hlsl_attribute_number_arguments_insufficient_shader_model: Error<
"attribute %0 with %1 arguments requires shader model %2 or greater">;
+def err_hlsl_expect_arg_const_int_one_or_neg_one: Error<
+ "argument %0 must be constant integer 1 or -1">;
// Layout randomization diagnostics.
def err_non_designated_init_used : Error<
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index e2d03eff8ab4a0..71273de3400b17 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -18959,6 +18959,14 @@ case Builtin::BI__builtin_hlsl_elementwise_isinf: {
CGM.getHLSLRuntime().getRadiansIntrinsic(), ArrayRef<Value *>{Op0},
nullptr, "hlsl.radians");
}
+ case Builtin::BI__builtin_hlsl_buffer_update_counter: {
+ Value *ResHandle = EmitScalarExpr(E->getArg(0));
+ Value *Offset = EmitScalarExpr(E->getArg(1));
+ return Builder.CreateIntrinsic(
+ /*ReturnType=*/Offset->getType(),
+ CGM.getHLSLRuntime().getBufferUpdateCounterIntrinsic(),
+ ArrayRef<Value *>{ResHandle, Offset}, nullptr);
+ }
}
return nullptr;
}
diff --git a/clang/lib/CodeGen/CGHLSLRuntime.h
b/clang/lib/CodeGen/CGHLSLRuntime.h
index ff7df41b5c62e7..aac93dfc373ed4 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.h
+++ b/clang/lib/CodeGen/CGHLSLRuntime.h
@@ -93,6 +93,7 @@ class CGHLSLRuntime {
GENERATE_HLSL_INTRINSIC_FUNCTION(WaveReadLaneAt, wave_readlane)
GENERATE_HLSL_INTRINSIC_FUNCTION(CreateHandleFromBinding, handle_fromBinding)
+ GENERATE_HLSL_INTRINSIC_FUNCTION(BufferUpdateCounter, bufferUpdateCounter)
//===----------------------------------------------------------------------===//
// End of reserved area for HLSL intrinsic getters.
diff --git a/clang/lib/Sema/HLSLExternalSemaSource.cpp
b/clang/lib/Sema/HLSLExternalSemaSource.cpp
index ce8564429b3802..24c3954b134c5f 100644
--- a/clang/lib/Sema/HLSLExternalSemaSource.cpp
+++ b/clang/lib/Sema/HLSLExternalSemaSource.cpp
@@ -22,12 +22,15 @@
#include "clang/Sema/SemaHLSL.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Frontend/HLSL/HLSLResource.h"
+#include "llvm/Support/ErrorHandling.h"
#include <functional>
using namespace clang;
using namespace llvm::hlsl;
+static DeclRefExpr *lookupBuiltinFunction(Sema &S, StringRef Name);
+
namespace {
struct TemplateParameterListBuilder;
@@ -121,12 +124,8 @@ struct BuiltinTypeDeclBuilder {
TypeSourceInfo *ElementTypeInfo = nullptr;
QualType ElemTy = Ctx.Char8Ty;
- if (Template) {
- if (const auto *TTD = dyn_cast<TemplateTypeParmDecl>(
- Template->getTemplateParameters()->getParam(0))) {
- ElemTy = QualType(TTD->getTypeForDecl(), 0);
- }
- }
+ if (Template)
+ ElemTy = getFirstTemplateTypeParam();
ElementTypeInfo = Ctx.getTrivialTypeSourceInfo(ElemTy, SourceLocation());
// add handle member with resource type attributes
@@ -145,25 +144,6 @@ struct BuiltinTypeDeclBuilder {
return *this;
}
- static DeclRefExpr *lookupBuiltinFunction(ASTContext &AST, Sema &S,
- StringRef Name) {
- IdentifierInfo &II = AST.Idents.get(Name, tok::TokenKind::identifier);
- DeclarationNameInfo NameInfo =
- DeclarationNameInfo(DeclarationName(&II), SourceLocation());
- LookupResult R(S, NameInfo, Sema::LookupOrdinaryName);
- // AllowBuiltinCreation is false but LookupDirect will create
- // the builtin when searching the global scope anyways...
- S.LookupName(R, S.getCurScope());
- // FIXME: If the builtin function was user-declared in global scope,
- // this assert *will* fail. Should this call LookupBuiltin instead?
- assert(R.isSingleResult() &&
- "Since this is a builtin it should always resolve!");
- auto *VD = cast<ValueDecl>(R.getFoundDecl());
- QualType Ty = VD->getType();
- return DeclRefExpr::Create(AST, NestedNameSpecifierLoc(), SourceLocation(),
- VD, false, NameInfo, Ty, VK_PRValue);
- }
-
static Expr *emitResourceClassExpr(ASTContext &AST, ResourceClass RC) {
return IntegerLiteral::Create(
AST,
@@ -211,12 +191,8 @@ struct BuiltinTypeDeclBuilder {
ASTContext &AST = Record->getASTContext();
QualType ElemTy = AST.Char8Ty;
- if (Template) {
- if (const auto *TTD = dyn_cast<TemplateTypeParmDecl>(
- Template->getTemplateParameters()->getParam(0))) {
- ElemTy = QualType(TTD->getTypeForDecl(), 0);
- }
- }
+ if (Template)
+ ElemTy = getFirstTemplateTypeParam();
QualType ReturnTy = ElemTy;
FunctionProtoType::ExtProtoInfo ExtInfo;
@@ -282,6 +258,23 @@ struct BuiltinTypeDeclBuilder {
return *this;
}
+ FieldDecl *getResourceHandleField() {
+ FieldDecl *FD = Fields["h"];
+ if (FD && FD->getType()->isHLSLAttributedResourceType())
+ return FD;
+ return nullptr;
+ }
+
+ QualType getFirstTemplateTypeParam() {
+ if (Template) {
+ if (const auto *TTD = dyn_cast<TemplateTypeParmDecl>(
+ Template->getTemplateParameters()->getParam(0))) {
+ return QualType(TTD->getTypeForDecl(), 0);
+ }
+ }
+ return QualType();
+ }
+
BuiltinTypeDeclBuilder &startDefinition() {
if (Record->isCompleteDefinition())
return *this;
@@ -302,6 +295,10 @@ struct BuiltinTypeDeclBuilder {
TemplateParameterListBuilder addTemplateArgumentList(Sema &S);
BuiltinTypeDeclBuilder &addSimpleTemplateParams(Sema &S,
ArrayRef<StringRef> Names);
+
+ // Builtin types methods
+ BuiltinTypeDeclBuilder &addIncrementCounterMethod(Sema &S);
+ BuiltinTypeDeclBuilder &addDecrementCounterMethod(Sema &S);
};
struct TemplateParameterListBuilder {
@@ -359,6 +356,176 @@ struct TemplateParameterListBuilder {
return Builder;
}
};
+
+// Builder for methods of builtin types. Allows adding methods to builtin types
+// using the builder pattern like this:
+//
+// BuiltinTypeMethodBuilder(Sema, RecordBuilder, "MethodName", ReturnType)
+// .addParam("param_name", Type, InOutModifier)
+// .callBuiltin("buildin_name", { BuiltinParams })
+// .finalizeMethod();
+//
+// The builder needs to have all of the method parameters before it can create
+// a CXXMethodDecl. It collects them in addParam calls and when a first
+// method that builds the body is called it creates the CXXMethodDecl and
+// ParmVarDecls instances. These can then be referenced from the body building
+// methods. Destructor or an explicit call to finalizeMethod() will complete
+// the method definition.
+struct BuiltinTypeMethodBuilder {
+ struct MethodParam {
+ const IdentifierInfo &NameII;
+ QualType Ty;
+ HLSLParamModifierAttr::Spelling Modifier;
+ MethodParam(const IdentifierInfo &NameII, QualType Ty,
+ HLSLParamModifierAttr::Spelling Modifier)
+ : NameII(NameII), Ty(Ty), Modifier(Modifier) {}
+ };
+
+ BuiltinTypeDeclBuilder &DeclBuilder;
+ Sema &S;
+ DeclarationNameInfo NameInfo;
+ QualType ReturnTy;
+ CXXMethodDecl *Method;
+ llvm::SmallVector<MethodParam> Params;
+ llvm::SmallVector<Stmt *> StmtsList;
+
+public:
+ BuiltinTypeMethodBuilder(Sema &S, BuiltinTypeDeclBuilder &DB, StringRef Name,
+ QualType ReturnTy)
+ : DeclBuilder(DB), S(S), ReturnTy(ReturnTy), Method(nullptr) {
+ const IdentifierInfo &II =
+ S.getASTContext().Idents.get(Name, tok::TokenKind::identifier);
+ NameInfo = DeclarationNameInfo(DeclarationName(&II), SourceLocation());
+ }
+
+ BuiltinTypeMethodBuilder &addParam(StringRef Name, QualType Ty,
+ HLSLParamModifierAttr::Spelling Modifier =
+ HLSLParamModifierAttr::Keyword_in) {
+ assert(Method == nullptr && "Cannot add param, method already created");
+
+ const IdentifierInfo &II =
+ S.getASTContext().Idents.get(Name, tok::TokenKind::identifier);
+ Params.emplace_back(II, Ty, Modifier);
+ return *this;
+ }
+
+private:
+ void createMethodDecl() {
+ assert(Method == nullptr && "Method already created");
+
+ // create method type
+ ASTContext &AST = S.getASTContext();
+ SmallVector<QualType> ParamTypes;
+ for (auto &MP : Params)
+ ParamTypes.emplace_back(MP.Ty);
+ QualType MethodTy = AST.getFunctionType(ReturnTy, ParamTypes,
+ FunctionProtoType::ExtProtoInfo());
+
+ // create method decl
+ auto *TSInfo = AST.getTrivialTypeSourceInfo(MethodTy, SourceLocation());
+ Method =
+ CXXMethodDecl::Create(AST, DeclBuilder.Record, SourceLocation(),
+ NameInfo, MethodTy, TSInfo, SC_None, false,
false,
+ ConstexprSpecKind::Unspecified,
SourceLocation());
+
+ // create params & set them to the function prototype
+ SmallVector<ParmVarDecl *> ParmDecls;
+ auto FnProtoLoc =
+
Method->getTypeSourceInfo()->getTypeLoc().getAs<FunctionProtoTypeLoc>();
+ unsigned i = 0;
+ for (auto &MP : Params) {
+ ParmVarDecl *Parm = ParmVarDecl::Create(
+ AST, Method->getDeclContext(), SourceLocation(), SourceLocation(),
+ &MP.NameII, MP.Ty,
+ AST.getTrivialTypeSourceInfo(MP.Ty, SourceLocation()), SC_None,
+ nullptr);
+ if (MP.Modifier != HLSLParamModifierAttr::Keyword_in) {
+ auto *Mod =
+ HLSLParamModifierAttr::Create(AST, SourceRange(), MP.Modifier);
+ Parm->addAttr(Mod);
+ }
+ ParmDecls.push_back(Parm);
+ FnProtoLoc.setParam(i++, Parm);
+ }
+ Method->setParams({ParmDecls});
+ }
+
+ void addResourceHandleToParms(SmallVector<Expr *> &Parms) {
+ ASTContext &AST = S.getASTContext();
+ FieldDecl *HandleField = DeclBuilder.getResourceHandleField();
+ auto *This = CXXThisExpr::Create(
+ AST, SourceLocation(), Method->getFunctionObjectParameterType(), true);
+ Parms.push_back(MemberExpr::CreateImplicit(AST, This, false, HandleField,
+ HandleField->getType(),
+ VK_LValue, OK_Ordinary));
+ }
+
+public:
+ ~BuiltinTypeMethodBuilder() { finalizeMethod(); }
+
+ BuiltinTypeMethodBuilder &
+ callBuiltin(StringRef BuiltinName, ArrayRef<Expr *> CallParms,
+ bool AddResourceHandleAsFirstArg = true) {
+ if (!Method)
+ createMethodDecl();
+
+ ASTContext &AST = S.getASTContext();
+ DeclRefExpr *Fn = lookupBuiltinFunction(S, BuiltinName);
+ Expr *Call = nullptr;
+
+ if (AddResourceHandleAsFirstArg) {
+ SmallVector<Expr *> NewCallParms;
+ addResourceHandleToParms(NewCallParms);
+ for (auto *P : CallParms)
+ NewCallParms.push_back(P);
+
+ Call = CallExpr::Create(AST, Fn, NewCallParms, AST.VoidPtrTy, VK_PRValue,
+ SourceLocation(), FPOptionsOverride());
+ } else {
+ Call = CallExpr::Create(AST, Fn, CallParms, AST.VoidPtrTy, VK_PRValue,
+ SourceLocation(), FPOptionsOverride());
+ }
+ StmtsList.push_back(Call);
+ return *this;
+ }
+
+ BuiltinTypeMethodBuilder &
+ callBuiltinForwardArgs(StringRef BuiltinName,
+ bool AddResourceHandleAsFirstArg = true) {
+ // FIXME: Call the buildin with all of the method parameters
+ // plus optional resource handle as the first arg.
+ llvm_unreachable("not yet implemented");
+ }
+
+ BuiltinTypeDeclBuilder &finalizeMethod() {
+ if (DeclBuilder.Record->isCompleteDefinition())
+ return DeclBuilder;
+
+ if (!Method)
+ createMethodDecl();
+
+ if (!Method->hasBody()) {
+ ASTContext &AST = S.getASTContext();
+ if (ReturnTy != AST.VoidTy && !StmtsList.empty()) {
+ if (Expr *LastExpr = dyn_cast<Expr>(StmtsList.back())) {
+ StmtsList.pop_back();
+ StmtsList.push_back(
+ ReturnStmt::Create(AST, SourceLocation(), LastExpr, nullptr));
+ }
+ }
+
+ Method->setBody(CompoundStmt::Create(AST, StmtsList, FPOptionsOverride(),
+ SourceLocation(),
SourceLocation()));
+ Method->setLexicalDeclContext(DeclBuilder.Record);
+ Method->setAccess(AccessSpecifier::AS_public);
+ Method->addAttr(AlwaysInlineAttr::CreateImplicit(
+ AST, SourceRange(), AlwaysInlineAttr::CXX11_clang_always_inline));
+ DeclBuilder.Record->addDecl(Method);
+ }
+ return DeclBuilder;
+ }
+};
+
} // namespace
TemplateParameterListBuilder
@@ -375,6 +542,30 @@ BuiltinTypeDeclBuilder::addSimpleTemplateParams(Sema &S,
return Builder.finalizeTemplateArgs();
}
+BuiltinTypeDeclBuilder &
+BuiltinTypeDeclBuilder::addIncrementCounterMethod(Sema &S) {
+ ASTContext &AST = S.getASTContext();
+ Expr *One =
+ IntegerLiteral::Create(AST, llvm::APInt(AST.getTypeSize(AST.IntTy), 1),
+ AST.IntTy, SourceLocation());
+ return BuiltinTypeMethodBuilder(S, *this, "IncrementCounter",
+ AST.UnsignedIntTy)
+ .callBuiltin("__builtin_hlsl_buffer_update_counter", {One})
+ .finalizeMethod();
+}
+
+BuiltinTypeDeclBuilder &
+BuiltinTypeDeclBuilder::addDecrementCounterMethod(Sema &S) {
+ ASTContext &AST = S.getASTContext();
+ Expr *NegOne =
+ IntegerLiteral::Create(AST, llvm::APInt(AST.getTypeSize(AST.IntTy), -1),
+ AST.IntTy, SourceLocation());
+ return BuiltinTypeMethodBuilder(S, *this, "DecrementCounter",
+ AST.UnsignedIntTy)
+ .callBuiltin("__builtin_hlsl_buffer_update_counter", {NegOne})
+ .finalizeMethod();
+}
+
HLSLExternalSemaSource::~HLSLExternalSemaSource() {}
void HLSLExternalSemaSource::InitializeSema(Sema &S) {
@@ -528,8 +719,13 @@ void
HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
ResourceKind::TypedBuffer, /*IsROV=*/false,
/*RawBuffer=*/true)
.addArraySubscriptOperators()
+ .addIncrementCounterMethod(*SemaPtr)
+ .addDecrementCounterMethod(*SemaPtr)
.completeDefinition();
});
+
+ // FIXME: Also add Increment/DecrementCounter to
+ // RasterizerOrderedStructuredBuffer when llvm/llvm-project/#113648 is
merged.
}
void HLSLExternalSemaSource::onCompletion(CXXRecordDecl *Record,
@@ -552,3 +748,23 @@ void HLSLExternalSemaSource::CompleteType(TagDecl *Tag) {
return;
It->second(Record);
}
+
+static DeclRefExpr *lookupBuiltinFunction(Sema &S, StringRef Name) {
+ IdentifierInfo &II =
+ S.getASTContext().Idents.get(Name, tok::TokenKind::identifier);
+ DeclarationNameInfo NameInfo =
+ DeclarationNameInfo(DeclarationName(&II), SourceLocation());
+ LookupResult R(S, NameInfo, Sema::LookupOrdinaryName);
+ // AllowBuiltinCreation is false but LookupDirect will create
+ // the builtin when searching the global scope anyways...
+ S.LookupName(R, S.getCurScope());
+ // FIXME: If the builtin function was user-declared in global scope,
+ // this assert *will* fail. Should this call LookupBuiltin instead?
+ assert(R.isSingleResult() &&
+ "Since this is a builtin it should always resolve!");
+ auto *VD = cast<ValueDecl>(R.getFoundDecl());
+ QualType Ty = VD->getType();
+ return DeclRefExpr::Create(S.getASTContext(), NestedNameSpecifierLoc(),
+ SourceLocation(), VD, false, NameInfo, Ty,
+ VK_PRValue);
+}
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index ff6616901016ab..770bd4a81633e1 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -986,6 +986,10 @@ Sema::VarArgKind Sema::isValidVarArgType(const QualType
&Ty) {
if (getLangOpts().MSVCCompat)
return VAK_MSVCUndefined;
+ if (getLangOpts().HLSL &&
+ Ty->getUnqualifiedDesugaredType()->isHLSLAttributedResourceType())
+ return VAK_Valid;
+
// FIXME: In C++11, these cases are conditionally-supported, meaning we're
// permitted to reject them. We should consider doing so.
return VAK_Undefined;
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index 1f6c5b8d4561bc..1b7f0456a3e82a 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -1860,6 +1860,31 @@ static bool CheckVectorSelect(Sema *S, CallExpr
*TheCall) {
return false;
}
+static bool CheckResourceHandle(Sema *S, CallExpr *TheCall, unsigned ArgIndex)
{
+ assert(TheCall->getNumArgs() >= ArgIndex);
+ QualType ArgType = TheCall->getArg(ArgIndex)->getType();
+ if (!ArgType.getTypePtr()
+ ->getUnqualifiedDesugaredType()
+ ->isHLSLAttributedResourceType()) {
+ S->Diag(TheCall->getArg(0)->getBeginLoc(),
+ diag::err_typecheck_expect_hlsl_resource)
+ << ArgType;
+ return true;
+ }
+ return false;
+}
+
+static bool CheckInt(Sema *S, CallExpr *TheCall, unsigned ArgIndex) {
+ assert(TheCall->getNumArgs() >= ArgIndex);
+ QualType ArgType = TheCall->getArg(ArgIndex)->getType();
+ if (!ArgType->isIntegerType()) {
+ S->Diag(TheCall->getArg(0)->getBeginLoc(), diag::err_typecheck_expect_int)
+ << ArgType;
+ return true;
+ }
+ return false;
+}
+
// Note: returning true in this case results in CheckBuiltinFunctionCall
// returning an ExprError
bool SemaHLSL::CheckBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall)
{
@@ -2100,6 +2125,22 @@ bool SemaHLSL::CheckBuiltinFunctionCall(unsigned
BuiltinID, CallExpr *TheCall) {
return true;
break;
}
+ case Builtin::BI__builtin_hlsl_buffer_update_counter: {
+ if (SemaRef.checkArgCount(TheCall, 2) ||
+ CheckResourceHandle(&SemaRef, TheCall, 0) ||
+ CheckInt(&SemaRef, TheCall, 1))
+ return true;
+ Expr *OffsetExpr = TheCall->getArg(1);
+ std::optional<llvm::APSInt> Offset =
+ OffsetExpr->getIntegerConstantExpr(SemaRef.getASTContext());
+ if (!Offset.has_value() || abs(Offset->getExtValue()) != 1) {
+ SemaRef.Diag(TheCall->getArg(1)->getBeginLoc(),
+ diag::err_hlsl_expect_arg_const_int_one_or_neg_one)
+ << 1;
+ return true;
+ }
+ break;
+ }
}
return false;
}
diff --git a/clang/test/CodeGenHLSL/builtins/StructuredBuffers-methods-lib.hlsl
b/clang/test/CodeGenHLSL/builtins/StructuredBuffers-methods-lib.hlsl
new file mode 100644
index 00000000000000..c8ff5d3cd905fb
--- /dev/null
+++ b/clang/test/CodeGenHLSL/builtins/StructuredBuffers-methods-lib.hlsl
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -emit-llvm
-disable-llvm-passes -o - %s | FileCheck %s --check-prefixes=CHECK,CHECK-DXIL
+// RUN-DISABLED: %clang_cc1 -triple spirv-vulkan-library -x hlsl -emit-llvm
-disable-llvm-passes -o - %s | FileCheck %s --check-prefixes=CHECK,CHECK-SPIRV
+
+// NOTE: SPIRV codegen for resource methods is not yet implemented
+
+RWStructuredBuffer<float> RWSB1 : register(u0);
+RWStructuredBuffer<float> RWSB2 : register(u1);
+
+// CHECK: %"class.hlsl::RWStructuredBuffer" = type { target("dx.RawBuffer",
float, 1, 0), float }
+
+export void TestIncrementCounter() {
+ RWSB1.IncrementCounter();
+}
+
+// CHECK: define void @_Z20TestIncrementCounterv()
+// CHECK-DXIL: call i32
@llvm.dx.bufferUpdateCounter.tdx.RawBuffer_f32_1_0t(target("dx.RawBuffer",
float, 1, 0) %{{[0-9]+}}, i32 1)
+
+export void TestDecrementCounter() {
+ RWSB2.DecrementCounter();
+}
+
+// CHECK: define void @_Z20TestDecrementCounterv()
+// CHECK-DXIL: call i32
@llvm.dx.bufferUpdateCounter.tdx.RawBuffer_f32_1_0t(target("dx.RawBuffer",
float, 1, 0) %{{[0-9]+}}, i32 -1)
+
+// CHECK: declare i32
@llvm.dx.bufferUpdateCounter.tdx.RawBuffer_f32_1_0t(target("dx.RawBuffer",
float, 1, 0), i32)
\ No newline at end of file
diff --git a/clang/test/CodeGenHLSL/builtins/StructuredBuffers-methods-ps.hlsl
b/clang/test/CodeGenHLSL/builtins/StructuredBuffers-methods-ps.hlsl
new file mode 100644
index 00000000000000..fe9e9cfdcb8736
--- /dev/null
+++ b/clang/test/CodeGenHLSL/builtins/StructuredBuffers-methods-ps.hlsl
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-pixel -x hlsl -emit-llvm
-disable-llvm-passes -o - %s | FileCheck %s --check-prefixes=CHECK,CHECK-DXIL
+// RUN-DISABLED: %clang_cc1 -triple spirv-vulkan-pixel -x hlsl -emit-llvm
-disable-llvm-passes -o - %s | FileCheck %s --check-prefixes=CHECK,CHECK-SPIRV
+
+// NOTE: SPIRV codegen for resource methods is not yet implemented
+
+RWStructuredBuffer<float> RWSB1, RWSB2;
+RasterizerOrderedStructuredBuffer<float> ROSB1, ROSB2;
+
+// CHECK: %"class.hlsl::RWStructuredBuffer" = type { target("dx.RawBuffer",
float, 1, 0), float }
+
+export void TestIncrementCounter() {
+ RWSB1.IncrementCounter();
+ ROSB1.IncrementCounter();
+}
+
+// CHECK: define void @_Z20TestIncrementCounterv()
+// CHECK-DXIL: call i32
@llvm.dx.bufferUpdateCounter.tdx.RawBuffer_f32_1_0t(target("dx.RawBuffer",
float, 1, 0) %{{[0-9]+}}, i32 1)
+// CHECK-DXIL: call i32
@llvm.dx.bufferUpdateCounter.tdx.RawBuffer_f32_1_0t(target("dx.RawBuffer",
float, 1, 1) %{{[0-9]+}}, i32 1)
+
+export void TestDecrementCounter() {
+ RWSB2.DecrementCounter();
+ ROSB2.DecrementCounter();
+}
+
+// CHECK: define void @_Z20TestDecrementCounterv()
+// CHECK-DXIL: call i32
@llvm.dx.bufferUpdateCounter.tdx.RawBuffer_f32_1_0t(target("dx.RawBuffer",
float, 1, 0) %{{[0-9]+}}, i32 -1)
+// CHECK-DXIL: call i32
@llvm.dx.bufferUpdateCounter.tdx.RawBuffer_f32_1_0t(target("dx.RawBuffer",
float, 1, 1) %{{[0-9]+}}, i32 -1)
+
+// CHECK: declare i32
@llvm.dx.bufferUpdateCounter.tdx.RawBuffer_f32_1_0t(target("dx.RawBuffer",
float, 1, 0), i32)
\ No newline at end of file
diff --git a/clang/test/SemaHLSL/BuiltIns/buffer_update_counter-errors.hlsl
b/clang/test/SemaHLSL/BuiltIns/buffer_update_counter-errors.hlsl
new file mode 100644
index 00000000000000..11b8cebc1aeb42
--- /dev/null
+++ b/clang/test/SemaHLSL/BuiltIns/buffer_update_counter-errors.hlsl
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -finclude-default-header -triple
dxil-pc-shadermodel6.6-library %s -emit-llvm-only -disable-llvm-passes -verify
+
+using handle_t = __hlsl_resource_t [[hlsl::resource_class(UAV)]]
[[hlsl::contained_type(int)]];
+
+void test_args(int x, bool b) {
+ handle_t res;
+
+ // expected-error@+1 {{too few arguments to function call, expected 2, have
1}}
+ __builtin_hlsl_buffer_update_counter(x);
+
+ // expected-error@+1 {{too many arguments to function call, expected 2, have
3}}
+ __builtin_hlsl_buffer_update_counter(x, x, x);
+
+ // expected-error@+1 {{used type 'int' where __hlsl_resource_t is required}}
+ __builtin_hlsl_buffer_update_counter(x, x);
+
+ // expected-error@+1 {{argument 1 must be constant integer 1 or -1}}
+ __builtin_hlsl_buffer_update_counter(res, x);
+
+ // expected-error@+1 {{argument 1 must be constant integer 1 or -1}}
+ __builtin_hlsl_buffer_update_counter(res, 10);
+}
diff --git a/llvm/include/llvm/IR/IntrinsicsDirectX.td
b/llvm/include/llvm/IR/IntrinsicsDirectX.td
index e30d37f69f781e..75e70657084bd0 100644
--- a/llvm/include/llvm/IR/IntrinsicsDirectX.td
+++ b/llvm/include/llvm/IR/IntrinsicsDirectX.td
@@ -35,6 +35,9 @@ def int_dx_typedBufferLoad_checkbit
def int_dx_typedBufferStore
: DefaultAttrsIntrinsic<[], [llvm_any_ty, llvm_i32_ty, llvm_anyvector_ty]>;
+def int_dx_bufferUpdateCounter
+ : DefaultAttrsIntrinsic<[llvm_i32_ty], [llvm_any_ty, llvm_i32_ty],
[IntrWriteMem]>;
+
// Cast between target extension handle types and dxil-style opaque handles
def int_dx_cast_handle : Intrinsic<[llvm_any_ty], [llvm_any_ty]>;
diff --git a/llvm/include/llvm/IR/IntrinsicsSPIRV.td
b/llvm/include/llvm/IR/IntrinsicsSPIRV.td
index 6df2eb156a0774..8978877feb69d6 100644
--- a/llvm/include/llvm/IR/IntrinsicsSPIRV.td
+++ b/llvm/include/llvm/IR/IntrinsicsSPIRV.td
@@ -97,4 +97,7 @@ let TargetPrefix = "spv" in {
[llvm_any_ty],
[llvm_i32_ty, llvm_i32_ty, llvm_i32_ty, llvm_i32_ty, llvm_i1_ty],
[IntrNoMem]>;
+
+ def int_spv_bufferUpdateCounter
+ : DefaultAttrsIntrinsic<[llvm_i32_ty], [llvm_any_ty, llvm_i32_ty],
[IntrWriteMem]>;
}
>From a94c90110d042c5170e8b8d469765688be657d2c Mon Sep 17 00:00:00 2001
From: Helena Kotas <heko...@microsoft.com>
Date: Thu, 31 Oct 2024 20:49:45 -0700
Subject: [PATCH 2/5] Code review feedback - add test & asserts, add Sema to
main decl builder, fix generated CallExpr type
---
clang/lib/Sema/HLSLExternalSemaSource.cpp | 147 +++++++++---------
.../test/AST/HLSL/RWStructuredBuffer-AST.hlsl | 26 ++++
2 files changed, 99 insertions(+), 74 deletions(-)
diff --git a/clang/lib/Sema/HLSLExternalSemaSource.cpp
b/clang/lib/Sema/HLSLExternalSemaSource.cpp
index 24c3954b134c5f..10814f10cfeb6e 100644
--- a/clang/lib/Sema/HLSLExternalSemaSource.cpp
+++ b/clang/lib/Sema/HLSLExternalSemaSource.cpp
@@ -12,7 +12,9 @@
#include "clang/Sema/HLSLExternalSemaSource.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Attr.h"
+#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
+#include "clang/AST/Expr.h"
#include "clang/AST/Type.h"
#include "clang/Basic/AttrKinds.h"
#include "clang/Basic/HLSLRuntime.h"
@@ -29,26 +31,27 @@
using namespace clang;
using namespace llvm::hlsl;
-static DeclRefExpr *lookupBuiltinFunction(Sema &S, StringRef Name);
+static FunctionDecl *lookupBuiltinFunction(Sema &S, StringRef Name);
namespace {
struct TemplateParameterListBuilder;
struct BuiltinTypeDeclBuilder {
+ Sema &S;
CXXRecordDecl *Record = nullptr;
ClassTemplateDecl *Template = nullptr;
ClassTemplateDecl *PrevTemplate = nullptr;
NamespaceDecl *HLSLNamespace = nullptr;
llvm::StringMap<FieldDecl *> Fields;
- BuiltinTypeDeclBuilder(CXXRecordDecl *R) : Record(R) {
+ BuiltinTypeDeclBuilder(Sema &S, CXXRecordDecl *R) : S(S), Record(R) {
Record->startDefinition();
Template = Record->getDescribedClassTemplate();
}
BuiltinTypeDeclBuilder(Sema &S, NamespaceDecl *Namespace, StringRef Name)
- : HLSLNamespace(Namespace) {
+ : S(S), HLSLNamespace(Namespace) {
ASTContext &AST = S.getASTContext();
IdentifierInfo &II = AST.Idents.get(Name, tok::TokenKind::identifier);
@@ -114,8 +117,7 @@ struct BuiltinTypeDeclBuilder {
}
BuiltinTypeDeclBuilder &
- addHandleMember(Sema &S, ResourceClass RC, ResourceKind RK, bool IsROV,
- bool RawBuffer,
+ addHandleMember(ResourceClass RC, ResourceKind RK, bool IsROV, bool
RawBuffer,
AccessSpecifier Access = AccessSpecifier::AS_private) {
if (Record->isCompleteDefinition())
return *this;
@@ -152,8 +154,7 @@ struct BuiltinTypeDeclBuilder {
AST.UnsignedCharTy, SourceLocation());
}
- BuiltinTypeDeclBuilder &addDefaultHandleConstructor(Sema &S,
- ResourceClass RC) {
+ BuiltinTypeDeclBuilder &addDefaultHandleConstructor(ResourceClass RC) {
if (Record->isCompleteDefinition())
return *this;
ASTContext &AST = Record->getASTContext();
@@ -260,22 +261,22 @@ struct BuiltinTypeDeclBuilder {
FieldDecl *getResourceHandleField() {
FieldDecl *FD = Fields["h"];
- if (FD && FD->getType()->isHLSLAttributedResourceType())
- return FD;
- return nullptr;
+ assert(FD && FD->getType()->isHLSLAttributedResourceType() &&
+ "record does not have resource handle");
+ return FD;
}
QualType getFirstTemplateTypeParam() {
- if (Template) {
- if (const auto *TTD = dyn_cast<TemplateTypeParmDecl>(
- Template->getTemplateParameters()->getParam(0))) {
- return QualType(TTD->getTypeForDecl(), 0);
- }
+ assert(Template && "record it not a template");
+ if (const auto *TTD = dyn_cast<TemplateTypeParmDecl>(
+ Template->getTemplateParameters()->getParam(0))) {
+ return QualType(TTD->getTypeForDecl(), 0);
}
return QualType();
}
BuiltinTypeDeclBuilder &startDefinition() {
+ // we might already have complete definition from a precompiled header
if (Record->isCompleteDefinition())
return *this;
Record->startDefinition();
@@ -292,22 +293,19 @@ struct BuiltinTypeDeclBuilder {
return *this;
}
- TemplateParameterListBuilder addTemplateArgumentList(Sema &S);
- BuiltinTypeDeclBuilder &addSimpleTemplateParams(Sema &S,
- ArrayRef<StringRef> Names);
+ TemplateParameterListBuilder addTemplateArgumentList();
+ BuiltinTypeDeclBuilder &addSimpleTemplateParams(ArrayRef<StringRef> Names);
// Builtin types methods
- BuiltinTypeDeclBuilder &addIncrementCounterMethod(Sema &S);
- BuiltinTypeDeclBuilder &addDecrementCounterMethod(Sema &S);
+ BuiltinTypeDeclBuilder &addIncrementCounterMethod();
+ BuiltinTypeDeclBuilder &addDecrementCounterMethod();
};
struct TemplateParameterListBuilder {
BuiltinTypeDeclBuilder &Builder;
- Sema &S;
llvm::SmallVector<NamedDecl *> Params;
- TemplateParameterListBuilder(Sema &S, BuiltinTypeDeclBuilder &RB)
- : Builder(RB), S(S) {}
+ TemplateParameterListBuilder(BuiltinTypeDeclBuilder &RB) : Builder(RB) {}
~TemplateParameterListBuilder() { finalizeTemplateArgs(); }
@@ -315,17 +313,18 @@ struct TemplateParameterListBuilder {
addTypeParameter(StringRef Name, QualType DefaultValue = QualType()) {
if (Builder.Record->isCompleteDefinition())
return *this;
+ ASTContext &AST = Builder.S.getASTContext();
unsigned Position = static_cast<unsigned>(Params.size());
auto *Decl = TemplateTypeParmDecl::Create(
- S.Context, Builder.Record->getDeclContext(), SourceLocation(),
+ AST, Builder.Record->getDeclContext(), SourceLocation(),
SourceLocation(), /* TemplateDepth */ 0, Position,
- &S.Context.Idents.get(Name, tok::TokenKind::identifier),
+ &AST.Idents.get(Name, tok::TokenKind::identifier),
/* Typename */ false,
/* ParameterPack */ false);
if (!DefaultValue.isNull())
Decl->setDefaultArgument(
- S.Context, S.getTrivialTemplateArgumentLoc(DefaultValue, QualType(),
- SourceLocation()));
+ AST, Builder.S.getTrivialTemplateArgumentLoc(DefaultValue,
QualType(),
+ SourceLocation()));
Params.emplace_back(Decl);
return *this;
@@ -334,11 +333,12 @@ struct TemplateParameterListBuilder {
BuiltinTypeDeclBuilder &finalizeTemplateArgs() {
if (Params.empty())
return Builder;
- auto *ParamList = TemplateParameterList::Create(S.Context,
SourceLocation(),
- SourceLocation(), Params,
- SourceLocation(), nullptr);
+ ASTContext &AST = Builder.S.Context;
+ auto *ParamList =
+ TemplateParameterList::Create(AST, SourceLocation(), SourceLocation(),
+ Params, SourceLocation(), nullptr);
Builder.Template = ClassTemplateDecl::Create(
- S.Context, Builder.Record->getDeclContext(), SourceLocation(),
+ AST, Builder.Record->getDeclContext(), SourceLocation(),
DeclarationName(Builder.Record->getIdentifier()), ParamList,
Builder.Record);
Builder.Record->setDescribedClassTemplate(Builder.Template);
@@ -351,7 +351,7 @@ struct TemplateParameterListBuilder {
Params.clear();
QualType T = Builder.Template->getInjectedClassNameSpecialization();
- T = S.Context.getInjectedClassNameType(Builder.Record, T);
+ T = AST.getInjectedClassNameType(Builder.Record, T);
return Builder;
}
@@ -382,7 +382,6 @@ struct BuiltinTypeMethodBuilder {
};
BuiltinTypeDeclBuilder &DeclBuilder;
- Sema &S;
DeclarationNameInfo NameInfo;
QualType ReturnTy;
CXXMethodDecl *Method;
@@ -392,7 +391,7 @@ struct BuiltinTypeMethodBuilder {
public:
BuiltinTypeMethodBuilder(Sema &S, BuiltinTypeDeclBuilder &DB, StringRef Name,
QualType ReturnTy)
- : DeclBuilder(DB), S(S), ReturnTy(ReturnTy), Method(nullptr) {
+ : DeclBuilder(DB), ReturnTy(ReturnTy), Method(nullptr) {
const IdentifierInfo &II =
S.getASTContext().Idents.get(Name, tok::TokenKind::identifier);
NameInfo = DeclarationNameInfo(DeclarationName(&II), SourceLocation());
@@ -403,8 +402,8 @@ struct BuiltinTypeMethodBuilder {
HLSLParamModifierAttr::Keyword_in) {
assert(Method == nullptr && "Cannot add param, method already created");
- const IdentifierInfo &II =
- S.getASTContext().Idents.get(Name, tok::TokenKind::identifier);
+ const IdentifierInfo &II = DeclBuilder.S.getASTContext().Idents.get(
+ Name, tok::TokenKind::identifier);
Params.emplace_back(II, Ty, Modifier);
return *this;
}
@@ -414,7 +413,7 @@ struct BuiltinTypeMethodBuilder {
assert(Method == nullptr && "Method already created");
// create method type
- ASTContext &AST = S.getASTContext();
+ ASTContext &AST = DeclBuilder.S.getASTContext();
SmallVector<QualType> ParamTypes;
for (auto &MP : Params)
ParamTypes.emplace_back(MP.Ty);
@@ -451,7 +450,7 @@ struct BuiltinTypeMethodBuilder {
}
void addResourceHandleToParms(SmallVector<Expr *> &Parms) {
- ASTContext &AST = S.getASTContext();
+ ASTContext &AST = DeclBuilder.S.getASTContext();
FieldDecl *HandleField = DeclBuilder.getResourceHandleField();
auto *This = CXXThisExpr::Create(
AST, SourceLocation(), Method->getFunctionObjectParameterType(), true);
@@ -469,22 +468,22 @@ struct BuiltinTypeMethodBuilder {
if (!Method)
createMethodDecl();
- ASTContext &AST = S.getASTContext();
- DeclRefExpr *Fn = lookupBuiltinFunction(S, BuiltinName);
- Expr *Call = nullptr;
+ ASTContext &AST = DeclBuilder.S.getASTContext();
+ FunctionDecl *FD = lookupBuiltinFunction(DeclBuilder.S, BuiltinName);
+ DeclRefExpr *DRE = DeclRefExpr::Create(
+ AST, NestedNameSpecifierLoc(), SourceLocation(), FD, false,
+ FD->getNameInfo(), FD->getType(), VK_PRValue);
+ SmallVector<Expr *> NewCallParms;
if (AddResourceHandleAsFirstArg) {
- SmallVector<Expr *> NewCallParms;
addResourceHandleToParms(NewCallParms);
for (auto *P : CallParms)
NewCallParms.push_back(P);
-
- Call = CallExpr::Create(AST, Fn, NewCallParms, AST.VoidPtrTy, VK_PRValue,
- SourceLocation(), FPOptionsOverride());
- } else {
- Call = CallExpr::Create(AST, Fn, CallParms, AST.VoidPtrTy, VK_PRValue,
- SourceLocation(), FPOptionsOverride());
}
+
+ Expr *Call = CallExpr::Create(
+ AST, DRE, AddResourceHandleAsFirstArg ? NewCallParms : CallParms,
+ FD->getReturnType(), VK_PRValue, SourceLocation(),
FPOptionsOverride());
StmtsList.push_back(Call);
return *this;
}
@@ -505,9 +504,16 @@ struct BuiltinTypeMethodBuilder {
createMethodDecl();
if (!Method->hasBody()) {
- ASTContext &AST = S.getASTContext();
+ ASTContext &AST = DeclBuilder.S.getASTContext();
if (ReturnTy != AST.VoidTy && !StmtsList.empty()) {
if (Expr *LastExpr = dyn_cast<Expr>(StmtsList.back())) {
+ assert(AST.hasSameUnqualifiedType(
+ isa<CallExpr>(LastExpr)
+ ? cast<CallExpr>(LastExpr)->getCallReturnType(AST)
+ : LastExpr->getType(),
+ ReturnTy) &&
+ "Return type of the last statement must match the return type
"
+ "of the method");
StmtsList.pop_back();
StmtsList.push_back(
ReturnStmt::Create(AST, SourceLocation(), LastExpr, nullptr));
@@ -528,22 +534,19 @@ struct BuiltinTypeMethodBuilder {
} // namespace
-TemplateParameterListBuilder
-BuiltinTypeDeclBuilder::addTemplateArgumentList(Sema &S) {
- return TemplateParameterListBuilder(S, *this);
+TemplateParameterListBuilder BuiltinTypeDeclBuilder::addTemplateArgumentList()
{
+ return TemplateParameterListBuilder(*this);
}
BuiltinTypeDeclBuilder &
-BuiltinTypeDeclBuilder::addSimpleTemplateParams(Sema &S,
- ArrayRef<StringRef> Names) {
- TemplateParameterListBuilder Builder = this->addTemplateArgumentList(S);
+BuiltinTypeDeclBuilder::addSimpleTemplateParams(ArrayRef<StringRef> Names) {
+ TemplateParameterListBuilder Builder = this->addTemplateArgumentList();
for (StringRef Name : Names)
Builder.addTypeParameter(Name);
return Builder.finalizeTemplateArgs();
}
-BuiltinTypeDeclBuilder &
-BuiltinTypeDeclBuilder::addIncrementCounterMethod(Sema &S) {
+BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addIncrementCounterMethod() {
ASTContext &AST = S.getASTContext();
Expr *One =
IntegerLiteral::Create(AST, llvm::APInt(AST.getTypeSize(AST.IntTy), 1),
@@ -554,8 +557,7 @@ BuiltinTypeDeclBuilder::addIncrementCounterMethod(Sema &S) {
.finalizeMethod();
}
-BuiltinTypeDeclBuilder &
-BuiltinTypeDeclBuilder::addDecrementCounterMethod(Sema &S) {
+BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addDecrementCounterMethod() {
ASTContext &AST = S.getASTContext();
Expr *NegOne =
IntegerLiteral::Create(AST, llvm::APInt(AST.getTypeSize(AST.IntTy), -1),
@@ -669,15 +671,15 @@ void HLSLExternalSemaSource::defineTrivialHLSLTypes() {
static BuiltinTypeDeclBuilder setupBufferType(CXXRecordDecl *Decl, Sema &S,
ResourceClass RC, ResourceKind
RK,
bool IsROV, bool RawBuffer) {
- return BuiltinTypeDeclBuilder(Decl)
- .addHandleMember(S, RC, RK, IsROV, RawBuffer)
- .addDefaultHandleConstructor(S, RC);
+ return BuiltinTypeDeclBuilder(S, Decl)
+ .addHandleMember(RC, RK, IsROV, RawBuffer)
+ .addDefaultHandleConstructor(RC);
}
void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
CXXRecordDecl *Decl;
Decl = BuiltinTypeDeclBuilder(*SemaPtr, HLSLNamespace, "RWBuffer")
- .addSimpleTemplateParams(*SemaPtr, {"element_type"})
+ .addSimpleTemplateParams({"element_type"})
.Record;
onCompletion(Decl, [this](CXXRecordDecl *Decl) {
@@ -690,7 +692,7 @@ void
HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
Decl =
BuiltinTypeDeclBuilder(*SemaPtr, HLSLNamespace,
"RasterizerOrderedBuffer")
- .addSimpleTemplateParams(*SemaPtr, {"element_type"})
+ .addSimpleTemplateParams({"element_type"})
.Record;
onCompletion(Decl, [this](CXXRecordDecl *Decl) {
setupBufferType(Decl, *SemaPtr, ResourceClass::UAV,
@@ -701,7 +703,7 @@ void
HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
});
Decl = BuiltinTypeDeclBuilder(*SemaPtr, HLSLNamespace, "StructuredBuffer")
- .addSimpleTemplateParams(*SemaPtr, {"element_type"})
+ .addSimpleTemplateParams({"element_type"})
.Record;
onCompletion(Decl, [this](CXXRecordDecl *Decl) {
setupBufferType(Decl, *SemaPtr, ResourceClass::SRV,
@@ -712,15 +714,15 @@ void
HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
});
Decl = BuiltinTypeDeclBuilder(*SemaPtr, HLSLNamespace, "RWStructuredBuffer")
- .addSimpleTemplateParams(*SemaPtr, {"element_type"})
+ .addSimpleTemplateParams({"element_type"})
.Record;
onCompletion(Decl, [this](CXXRecordDecl *Decl) {
setupBufferType(Decl, *SemaPtr, ResourceClass::UAV,
ResourceKind::TypedBuffer, /*IsROV=*/false,
/*RawBuffer=*/true)
.addArraySubscriptOperators()
- .addIncrementCounterMethod(*SemaPtr)
- .addDecrementCounterMethod(*SemaPtr)
+ .addIncrementCounterMethod()
+ .addDecrementCounterMethod()
.completeDefinition();
});
@@ -749,7 +751,7 @@ void HLSLExternalSemaSource::CompleteType(TagDecl *Tag) {
It->second(Record);
}
-static DeclRefExpr *lookupBuiltinFunction(Sema &S, StringRef Name) {
+static FunctionDecl *lookupBuiltinFunction(Sema &S, StringRef Name) {
IdentifierInfo &II =
S.getASTContext().Idents.get(Name, tok::TokenKind::identifier);
DeclarationNameInfo NameInfo =
@@ -762,9 +764,6 @@ static DeclRefExpr *lookupBuiltinFunction(Sema &S,
StringRef Name) {
// this assert *will* fail. Should this call LookupBuiltin instead?
assert(R.isSingleResult() &&
"Since this is a builtin it should always resolve!");
- auto *VD = cast<ValueDecl>(R.getFoundDecl());
- QualType Ty = VD->getType();
- return DeclRefExpr::Create(S.getASTContext(), NestedNameSpecifierLoc(),
- SourceLocation(), VD, false, NameInfo, Ty,
- VK_PRValue);
+ assert(isa<FunctionDecl>(R.getFoundDecl()));
+ return cast<FunctionDecl>(R.getFoundDecl());
}
diff --git a/clang/test/AST/HLSL/RWStructuredBuffer-AST.hlsl
b/clang/test/AST/HLSL/RWStructuredBuffer-AST.hlsl
index f95d74b30acded..fd283b1af0226f 100644
--- a/clang/test/AST/HLSL/RWStructuredBuffer-AST.hlsl
+++ b/clang/test/AST/HLSL/RWStructuredBuffer-AST.hlsl
@@ -52,6 +52,32 @@ RWStructuredBuffer<int> Buffer;
// CHECK-NEXT: CXXThisExpr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>>
'RWStructuredBuffer<element_type>' lvalue implicit this
// CHECK-NEXT: AlwaysInlineAttr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> Implicit
always_inline
+// CHECK-NEXT: CXXMethodDecl 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> <invalid
sloc> IncrementCounter 'unsigned int ()'
+// CHECK-NEXT: CompoundStmt 0x{{[0-9A-Fa-f]+}} <<invalid sloc>>
+// CHECK-NEXT: ReturnStmt 0x{{[0-9A-Fa-f]+}} <<invalid sloc>>
+// CHECK-NEXT: CallExpr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> 'unsigned int'
+// CHECK-NEXT: DeclRefExpr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> 'unsigned int
(...) noexcept' Function 0x{{[0-9A-Fa-f]+}}
'__builtin_hlsl_buffer_update_counter' 'unsigned int (...) noexcept'
+// CHECK-NEXT: MemberExpr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>>
'__hlsl_resource_t
+// CHECK-SAME{LITERAL}: [[hlsl::resource_class(UAV)]]
+// CHECK-SAME{LITERAL}: [[hlsl::raw_buffer]]
+// CHECK-SAME{LITERAL}: [[hlsl::contained_type(element_type)]]' lvalue .h
+// CHECK-NEXT: CXXThisExpr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>>
'RWStructuredBuffer<element_type>' lvalue implicit this
+// CHECK-NEXT: IntegerLiteral 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> 'int' 1
+// CHECK-NEXT: AlwaysInlineAttr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> Implicit
always_inline
+
+// CHECK-NEXT: CXXMethodDecl 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> <invalid
sloc> DecrementCounter 'unsigned int ()'
+// CHECK-NEXT: CompoundStmt 0x{{[0-9A-Fa-f]+}} <<invalid sloc>>
+// CHECK-NEXT: ReturnStmt 0x{{[0-9A-Fa-f]+}} <<invalid sloc>>
+// CHECK-NEXT: CallExpr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> 'unsigned int'
+// CHECK-NEXT: DeclRefExpr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> 'unsigned int
(...) noexcept' Function 0x{{[0-9A-Fa-f]+}}
'__builtin_hlsl_buffer_update_counter' 'unsigned int (...) noexcept'
+// CHECK-NEXT: MemberExpr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>>
'__hlsl_resource_t
+// CHECK-SAME{LITERAL}: [[hlsl::resource_class(UAV)]]
+// CHECK-SAME{LITERAL}: [[hlsl::raw_buffer]]
+// CHECK-SAME{LITERAL}: [[hlsl::contained_type(element_type)]]' lvalue .h
+// CHECK-NEXT: CXXThisExpr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>>
'RWStructuredBuffer<element_type>' lvalue implicit this
+// CHECK-NEXT: IntegerLiteral 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> 'int' -1
+// CHECK-NEXT: AlwaysInlineAttr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> Implicit
always_inline
+
// CHECK: ClassTemplateSpecializationDecl 0x{{[0-9A-Fa-f]+}} <<invalid sloc>>
<invalid sloc> class RWStructuredBuffer definition
// CHECK: TemplateArgument type 'int'
>From 0f4c61f3ab51c8e614e921e43827f0c28be74685 Mon Sep 17 00:00:00 2001
From: Helena Kotas <heko...@microsoft.com>
Date: Mon, 4 Nov 2024 16:13:03 -0800
Subject: [PATCH 3/5] Code review feedback - assert of completed definition or
no method body
Do not complete definition if we already got one from precompiled headers.
---
clang/lib/Sema/HLSLExternalSemaSource.cpp | 41 +++++++++++------------
1 file changed, 19 insertions(+), 22 deletions(-)
diff --git a/clang/lib/Sema/HLSLExternalSemaSource.cpp
b/clang/lib/Sema/HLSLExternalSemaSource.cpp
index 10814f10cfeb6e..52f1ec3bd39e3c 100644
--- a/clang/lib/Sema/HLSLExternalSemaSource.cpp
+++ b/clang/lib/Sema/HLSLExternalSemaSource.cpp
@@ -58,6 +58,7 @@ struct BuiltinTypeDeclBuilder {
LookupResult Result(S, &II, SourceLocation(), Sema::LookupTagName);
CXXRecordDecl *PrevDecl = nullptr;
if (S.LookupQualifiedName(Result, HLSLNamespace)) {
+ // Declaration already exists (from precompiled headers)
NamedDecl *Found = Result.getFoundDecl();
if (auto *TD = dyn_cast<ClassTemplateDecl>(Found)) {
PrevDecl = TD->getTemplatedDecl();
@@ -69,6 +70,7 @@ struct BuiltinTypeDeclBuilder {
if (PrevDecl && PrevDecl->isCompleteDefinition()) {
Record = PrevDecl;
+ Template = PrevTemplate;
return;
}
@@ -92,8 +94,7 @@ struct BuiltinTypeDeclBuilder {
BuiltinTypeDeclBuilder &
addMemberVariable(StringRef Name, QualType Type, llvm::ArrayRef<Attr *>
Attrs,
AccessSpecifier Access = AccessSpecifier::AS_private) {
- if (Record->isCompleteDefinition())
- return *this;
+ assert(!Record->isCompleteDefinition() && "record is already complete");
assert(Record->isBeingDefined() &&
"Definition must be started before adding members!");
ASTContext &AST = Record->getASTContext();
@@ -119,8 +120,7 @@ struct BuiltinTypeDeclBuilder {
BuiltinTypeDeclBuilder &
addHandleMember(ResourceClass RC, ResourceKind RK, bool IsROV, bool
RawBuffer,
AccessSpecifier Access = AccessSpecifier::AS_private) {
- if (Record->isCompleteDefinition())
- return *this;
+ assert(!Record->isCompleteDefinition() && "record is already complete");
ASTContext &Ctx = S.getASTContext();
TypeSourceInfo *ElementTypeInfo = nullptr;
@@ -155,8 +155,7 @@ struct BuiltinTypeDeclBuilder {
}
BuiltinTypeDeclBuilder &addDefaultHandleConstructor(ResourceClass RC) {
- if (Record->isCompleteDefinition())
- return *this;
+ assert(!Record->isCompleteDefinition() && "record is already complete");
ASTContext &AST = Record->getASTContext();
QualType ConstructorType =
@@ -179,16 +178,13 @@ struct BuiltinTypeDeclBuilder {
}
BuiltinTypeDeclBuilder &addArraySubscriptOperators() {
- if (Record->isCompleteDefinition())
- return *this;
addArraySubscriptOperator(true);
addArraySubscriptOperator(false);
return *this;
}
BuiltinTypeDeclBuilder &addArraySubscriptOperator(bool IsConst) {
- if (Record->isCompleteDefinition())
- return *this;
+ assert(!Record->isCompleteDefinition() && "record is already complete");
ASTContext &AST = Record->getASTContext();
QualType ElemTy = AST.Char8Ty;
@@ -277,15 +273,13 @@ struct BuiltinTypeDeclBuilder {
BuiltinTypeDeclBuilder &startDefinition() {
// we might already have complete definition from a precompiled header
- if (Record->isCompleteDefinition())
- return *this;
+ assert(!Record->isCompleteDefinition() && "record is already complete");
Record->startDefinition();
return *this;
}
BuiltinTypeDeclBuilder &completeDefinition() {
- if (Record->isCompleteDefinition())
- return *this;
+ assert(!Record->isCompleteDefinition() && "record is already complete");
assert(Record->isBeingDefined() &&
"Definition must be started before completing it.");
@@ -311,8 +305,7 @@ struct TemplateParameterListBuilder {
TemplateParameterListBuilder &
addTypeParameter(StringRef Name, QualType DefaultValue = QualType()) {
- if (Builder.Record->isCompleteDefinition())
- return *this;
+ assert(!Builder.Record->isCompleteDefinition() && "record is already
complete");
ASTContext &AST = Builder.S.getASTContext();
unsigned Position = static_cast<unsigned>(Params.size());
auto *Decl = TemplateTypeParmDecl::Create(
@@ -497,11 +490,8 @@ struct BuiltinTypeMethodBuilder {
}
BuiltinTypeDeclBuilder &finalizeMethod() {
- if (DeclBuilder.Record->isCompleteDefinition())
- return DeclBuilder;
-
- if (!Method)
- createMethodDecl();
+ assert(!DeclBuilder.Record->isCompleteDefinition() && "record is already
complete");
+ assert(Method != nullptr && "method decl not created; are you missing a
call to build the body?");
if (!Method->hasBody()) {
ASTContext &AST = DeclBuilder.S.getASTContext();
@@ -540,6 +530,12 @@ TemplateParameterListBuilder
BuiltinTypeDeclBuilder::addTemplateArgumentList() {
BuiltinTypeDeclBuilder &
BuiltinTypeDeclBuilder::addSimpleTemplateParams(ArrayRef<StringRef> Names) {
+ if (Record->isCompleteDefinition()) {
+ assert(Template && "existing record it not a template");
+ assert(Template->getTemplateParameters()->size() == Names.size() &&
"template param count mismatch");
+ return *this;
+ }
+
TemplateParameterListBuilder Builder = this->addTemplateArgumentList();
for (StringRef Name : Names)
Builder.addTypeParameter(Name);
@@ -732,7 +728,8 @@ void
HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
void HLSLExternalSemaSource::onCompletion(CXXRecordDecl *Record,
CompletionFunction Fn) {
- Completions.insert(std::make_pair(Record->getCanonicalDecl(), Fn));
+ if (!Record->isCompleteDefinition())
+ Completions.insert(std::make_pair(Record->getCanonicalDecl(), Fn));
}
void HLSLExternalSemaSource::CompleteType(TagDecl *Tag) {
>From f180391871e898851a221b7e120c137464b45b85 Mon Sep 17 00:00:00 2001
From: Helena Kotas <heko...@microsoft.com>
Date: Mon, 4 Nov 2024 16:17:23 -0800
Subject: [PATCH 4/5] clang-format
---
clang/lib/Sema/HLSLExternalSemaSource.cpp | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/clang/lib/Sema/HLSLExternalSemaSource.cpp
b/clang/lib/Sema/HLSLExternalSemaSource.cpp
index 52f1ec3bd39e3c..b64efb481146d4 100644
--- a/clang/lib/Sema/HLSLExternalSemaSource.cpp
+++ b/clang/lib/Sema/HLSLExternalSemaSource.cpp
@@ -305,7 +305,8 @@ struct TemplateParameterListBuilder {
TemplateParameterListBuilder &
addTypeParameter(StringRef Name, QualType DefaultValue = QualType()) {
- assert(!Builder.Record->isCompleteDefinition() && "record is already
complete");
+ assert(!Builder.Record->isCompleteDefinition() &&
+ "record is already complete");
ASTContext &AST = Builder.S.getASTContext();
unsigned Position = static_cast<unsigned>(Params.size());
auto *Decl = TemplateTypeParmDecl::Create(
@@ -490,8 +491,11 @@ struct BuiltinTypeMethodBuilder {
}
BuiltinTypeDeclBuilder &finalizeMethod() {
- assert(!DeclBuilder.Record->isCompleteDefinition() && "record is already
complete");
- assert(Method != nullptr && "method decl not created; are you missing a
call to build the body?");
+ assert(!DeclBuilder.Record->isCompleteDefinition() &&
+ "record is already complete");
+ assert(
+ Method != nullptr &&
+ "method decl not created; are you missing a call to build the body?");
if (!Method->hasBody()) {
ASTContext &AST = DeclBuilder.S.getASTContext();
@@ -532,7 +536,8 @@ BuiltinTypeDeclBuilder &
BuiltinTypeDeclBuilder::addSimpleTemplateParams(ArrayRef<StringRef> Names) {
if (Record->isCompleteDefinition()) {
assert(Template && "existing record it not a template");
- assert(Template->getTemplateParameters()->size() == Names.size() &&
"template param count mismatch");
+ assert(Template->getTemplateParameters()->size() == Names.size() &&
+ "template param count mismatch");
return *this;
}
>From 942af76a3d008d23e78c38d1a6bff82a865a0d90 Mon Sep 17 00:00:00 2001
From: Helena Kotas <heko...@microsoft.com>
Date: Tue, 5 Nov 2024 11:59:44 -0800
Subject: [PATCH 5/5] Code review feedback - remove unused method, add comment
---
clang/lib/Sema/HLSLExternalSemaSource.cpp | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/clang/lib/Sema/HLSLExternalSemaSource.cpp
b/clang/lib/Sema/HLSLExternalSemaSource.cpp
index b64efb481146d4..45302f86d4d724 100644
--- a/clang/lib/Sema/HLSLExternalSemaSource.cpp
+++ b/clang/lib/Sema/HLSLExternalSemaSource.cpp
@@ -459,6 +459,8 @@ struct BuiltinTypeMethodBuilder {
BuiltinTypeMethodBuilder &
callBuiltin(StringRef BuiltinName, ArrayRef<Expr *> CallParms,
bool AddResourceHandleAsFirstArg = true) {
+
+ // The first statement added to a method creates the declaration.
if (!Method)
createMethodDecl();
@@ -482,14 +484,6 @@ struct BuiltinTypeMethodBuilder {
return *this;
}
- BuiltinTypeMethodBuilder &
- callBuiltinForwardArgs(StringRef BuiltinName,
- bool AddResourceHandleAsFirstArg = true) {
- // FIXME: Call the buildin with all of the method parameters
- // plus optional resource handle as the first arg.
- llvm_unreachable("not yet implemented");
- }
-
BuiltinTypeDeclBuilder &finalizeMethod() {
assert(!DeclBuilder.Record->isCompleteDefinition() &&
"record is already complete");
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
