Skip to content

Commit

Permalink
Introduce HLSLCXXOverload attribute (#5018)
Browse files Browse the repository at this point in the history
This change should have no functional impact on compilation. The goal is
to remove cases where we rely on the subscript operator's name for
special resolution rules and instead key the behavior off an attribute
on the method declaration.

The added test cases are not exhaustive but try to cover at least one
object type exercising each code path modified in this change.

Also added a test case for HLSL 2021 user defined array operator. This
change partially addresses #4340. The remaining parts of that issue
will be addressed in a future HLSL language revision.
  • Loading branch information
llvm-beanz authored Feb 13, 2023
1 parent fb00139 commit aea5a49
Show file tree
Hide file tree
Showing 9 changed files with 221 additions and 30 deletions.
8 changes: 8 additions & 0 deletions tools/clang/include/clang/Basic/Attr.td
Original file line number Diff line number Diff line change
Expand Up @@ -917,6 +917,14 @@ def HLSLWaveOpsIncludeHelperLanes : InheritableAttr {
let Documentation = [Undocumented];
}

// This attribute has no spelling, it is attached to methods by the compiler for
// methods that require C++ lookup rules.
def HLSLCXXOverload : InheritableAttr {
let Spellings = [];
let Subjects = SubjectList<[CXXMethod]>;
let Documentation = [Undocumented];
}

// HLSL Change Ends

// SPIRV Change Starts
Expand Down
15 changes: 10 additions & 5 deletions tools/clang/lib/AST/ASTContextHLSL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -291,10 +291,11 @@ void AddSubscriptOperator(
vectorType = context.getConstType(vectorType);

QualType indexType = intType;
CreateObjectFunctionDeclarationWithParams(
context, templateRecordDecl, vectorType,
ArrayRef<QualType>(indexType), ArrayRef<StringRef>(StringRef("index")),
context.DeclarationNames.getCXXOperatorName(OO_Subscript), forConst);
auto methodDecl = CreateObjectFunctionDeclarationWithParams(
context, templateRecordDecl, vectorType, ArrayRef<QualType>(indexType),
ArrayRef<StringRef>(StringRef("index")),
context.DeclarationNames.getCXXOperatorName(OO_Subscript), forConst);
methodDecl->addAttr(HLSLCXXOverloadAttr::CreateImplicit(context));
}

/// <summary>Adds up-front support for HLSL matrix types (just the template declaration).</summary>
Expand Down Expand Up @@ -351,7 +352,10 @@ void hlsl::AddHLSLMatrixTemplate(ASTContext& context, ClassTemplateDecl* vectorT

static void AddHLSLVectorSubscriptAttr(Decl *D, ASTContext &context) {
StringRef group = GetHLOpcodeGroupName(HLOpcodeGroup::HLSubscript);
D->addAttr(HLSLIntrinsicAttr::CreateImplicit(context, group, "", static_cast<unsigned>(HLSubscriptOpcode::VectorSubscript)));
D->addAttr(HLSLIntrinsicAttr::CreateImplicit(
context, group, "",
static_cast<unsigned>(HLSubscriptOpcode::VectorSubscript)));
D->addAttr(HLSLCXXOverloadAttr::CreateImplicit(context));
}

/// <summary>Adds up-front support for HLSL vector types (just the template declaration).</summary>
Expand Down Expand Up @@ -941,6 +945,7 @@ CXXRecordDecl* hlsl::DeclareResourceType(ASTContext& context, bool bSampler) {
functionDecl->addAttr(HLSLIntrinsicAttr::CreateImplicit(
context, "op", "",
static_cast<int>(hlsl::IntrinsicOp::IOP_CreateResourceFromHeap)));
functionDecl->addAttr(HLSLCXXOverloadAttr::CreateImplicit(context));
return recordDecl;
}

Expand Down
8 changes: 6 additions & 2 deletions tools/clang/lib/Sema/SemaHLSL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2686,9 +2686,12 @@ class IntrinsicDefIter
}
};

static void AddHLSLSubscriptAttr(Decl *D, ASTContext &context, HLSubscriptOpcode opcode) {
static void AddHLSLSubscriptAttr(Decl *D, ASTContext &context,
HLSubscriptOpcode opcode) {
StringRef group = GetHLOpcodeGroupName(HLOpcodeGroup::HLSubscript);
D->addAttr(HLSLIntrinsicAttr::CreateImplicit(context, group, "", static_cast<unsigned>(opcode)));
D->addAttr(HLSLIntrinsicAttr::CreateImplicit(context, group, "",
static_cast<unsigned>(opcode)));
D->addAttr(HLSLCXXOverloadAttr::CreateImplicit(context));
}

static void CreateSimpleField(clang::ASTContext &context, CXXRecordDecl *recordDecl, StringRef Name,
Expand Down Expand Up @@ -3459,6 +3462,7 @@ class HLSLExternalSource : public ExternalSemaSource {
hlsl::CreateFunctionTemplateDecl(
*m_context, recordDecl, functionDecl,
reinterpret_cast<NamedDecl **>(&templateTypeParmDecl), 1);
functionDecl->addAttr(HLSLCXXOverloadAttr::CreateImplicit(*m_context));

// Add a .mips member if necessary.
QualType uintType = m_context->UnsignedIntTy;
Expand Down
28 changes: 16 additions & 12 deletions tools/clang/lib/Sema/SemaOverload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4869,16 +4869,20 @@ TryObjectArgumentInitialization(Sema &S, QualType FromType,

// First check the qualifiers.
QualType FromTypeCanon = S.Context.getCanonicalType(FromType);
// HLSL Change Starts - for calls other than subscript overloads, disregard const
// HLSL Change Starts
// HLSL Note: For calls that aren't compiler-generated C++ overloads, we
// disregard const qualifiers so that member functions can be called on
// `const` objects from constant buffer types. This should change in the
// future if we support const instance methods.
FromTypeCanon.removeLocalRestrict(); // HLSL Change - disregard restrict.
if (!S.getLangOpts().HLSL ||
(Method != nullptr && Method->getDeclName() == S.Context.DeclarationNames.getCXXOperatorName(OO_Subscript))) {
// HLSL Change Ends
if (ImplicitParamType.getCVRQualifiers()
!= FromTypeCanon.getLocalCVRQualifiers() &&
(Method != nullptr && Method->hasAttr<HLSLCXXOverloadAttr>())) {
// HLSL Change Ends
if (ImplicitParamType.getCVRQualifiers() !=
FromTypeCanon.getLocalCVRQualifiers() &&
!ImplicitParamType.isAtLeastAsQualifiedAs(FromTypeCanon)) {
ICS.setBad(BadConversionSequence::bad_qualifiers,
FromType, ImplicitParamType);
ICS.setBad(BadConversionSequence::bad_qualifiers, FromType,
ImplicitParamType);
return ICS;
}
} // HLSL Change - end branch
Expand Down Expand Up @@ -8664,12 +8668,12 @@ OverloadCandidateSet::BestViableFunction(Sema &S, SourceLocation Loc,
iterator &Best,
bool UserDefinedConversion) {
// HLSL Change Starts
// Function calls should use HLSL-style overloading. operator[] overloads
// (used for const support) aren't supported by the defined rules, so
// use C++ overload resolution for those.
// Function calls should use HLSL-style overloading. Except for compiler
// generated functions which are annotated as requiring C++ overload
// resolution like operator[] overloads where `const` methods are aren't
// supported by HLSL's defined rules.
if (S.getLangOpts().HLSL && !empty() && begin()->Function != nullptr &&
(begin()->Function->getDeclName() !=
S.Context.DeclarationNames.getCXXOperatorName(OO_Subscript))) {
!begin()->Function->hasAttr<HLSLCXXOverloadAttr>()) {
return ::hlsl::GetBestViableFunction(S, Loc, *this, Best);
}
// HLSL Change Ends
Expand Down
78 changes: 67 additions & 11 deletions tools/clang/test/HLSLFileCheck/hlsl/objects/Buffer/buf_index.hlsl
Original file line number Diff line number Diff line change
@@ -1,16 +1,72 @@
// RUN: %dxc -E main -T ps_6_0 %s | FileCheck %s

// Tests vector index on buffer load.
// CHECK:alloca [4 x float]
// CHECK:dx.op.bufferLoad.f32
// CHECK:store
// CHECK:store
// CHECK:store
// CHECK:store
// CHECK:load
// RUN: %dxc -E main -T ps_6_0 -ast-dump-implicit %s | FileCheck %s -check-prefix=AST
// RUN: %dxc -E main -T ps_6_0 -fcgl %s | FileCheck %s -check-prefix=FCGL
// RUN: %dxc -E main -T ps_6_0 %s | FileCheck %s -check-prefix=OPTIMIZED

Buffer<float4> buf;

float4 main(uint i:I) : SV_Target {
return buf[2][i];
}
}

// Tests vector index converting to an expected load sequence.

// Verify the generated AST for the subscript operator.

// AST: ClassTemplateDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> implicit Buffer
// AST: CXXRecordDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> implicit class Buffer definition
// AST-NOT: CXXRecordDecl

// This match is a little intentionally fuzzy. There's some oddities in DXC's
// ASTs which are a little tricky to resolve but we might want to fix someday.

// AST: CXXMethodDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> operator[] 'const element &(unsigned int) const'
// AST-NEXT: ParmVarDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> index 'unsigned int'
// AST-NEXT: HLSLCXXOverloadAttr {{0x[0-9a-fA-F]+}} <<invalid sloc>> Implicit

// Verify the initial IR generation correctly generates the handle and subscript
// intrinsics. These matches are some crazy long lines, so they are broken into
// multiple checks on the same line of text.

// FCGL: [[Buffer:%.+]] = load %"class.Buffer<vector<float, 4> >",
// FCGL-SAME: %"class.Buffer<vector<float, 4> >"* @"\01?buf@@3V?$Buffer@V?$vector@M$03@@@@A"

// FCGL: [[Handle:%.+]] = call %dx.types.Handle
// FCGL-SAME: @"dx.hl.createhandle..%dx.types.Handle (i32, %\22class.Buffer<vector<float, 4> >\22)"
// FCGL-SAME: (i32 0, %"class.Buffer<vector<float, 4> >" [[Buffer]])

// FCGL: [[AnnHandle:%.+]] = call %dx.types.Handle @"dx.hl.annotatehandle..%dx.types.Handle
// FCGL-SAME: (i32, %dx.types.Handle, %dx.types.ResourceProperties, %\22class.Buffer<vector<float, 4> >\22)"
// FCGL-SAME: (i32 {{[0-9]+}},
// FCGL-SAME: %dx.types.Handle [[Handle]],
// FCGL-SAME: %dx.types.ResourceProperties { i32 {{[0-9]+}}, i32 {{[0-9]+}} },
// FCGL-SAME: %"class.Buffer<vector<float, 4> >" undef)

// FCGL: {{%.+}} = call <4 x float>* @"dx.hl.subscript.[].<4 x float>* (i32, %dx.types.Handle, i32)"
// FCGL-SAME: (i32 0, %dx.types.Handle [[AnnHandle]], i32 2)


// Verifies the optimized and loowered DXIL representation.
// OPTIMIZED: [[tmp:%.+]] = alloca [4 x float]
// OPTIMIZED: [[loaded:%.+]] = call %dx.types.ResRet.f32 @dx.op.bufferLoad.f32(
// OPTIMIZED: [[X:%.+]] = extractvalue %dx.types.ResRet.f32 [[loaded]], 0
// OPTIMIZED: [[Y:%.+]] = extractvalue %dx.types.ResRet.f32 [[loaded]], 1
// OPTIMIZED: [[Z:%.+]] = extractvalue %dx.types.ResRet.f32 [[loaded]], 2
// OPTIMIZED: [[W:%.+]] = extractvalue %dx.types.ResRet.f32 [[loaded]], 3

// OPTIMIZED-DAG: [[XAddr:%.+]] = getelementptr inbounds [4 x float], [4 x float]* [[tmp]], i32 0, i32 0
// OPTIMIZED-DAG: [[YAddr:%.+]] = getelementptr inbounds [4 x float], [4 x float]* [[tmp]], i32 0, i32 1
// OPTIMIZED-DAG: [[ZAddr:%.+]] = getelementptr inbounds [4 x float], [4 x float]* [[tmp]], i32 0, i32 2
// OPTIMIZED-DAG: [[WAddr:%.+]] = getelementptr inbounds [4 x float], [4 x float]* [[tmp]], i32 0, i32 3

// OPTIMIZED-DAG: store float [[X]], float* [[XAddr]]
// OPTIMIZED-DAG: store float [[Y]], float* [[YAddr]]
// OPTIMIZED-DAG: store float [[Z]], float* [[ZAddr]]
// OPTIMIZED-DAG: store float [[W]], float* [[WAddr]]

// OPTIMIZED: [[elAddr:%.+]] = getelementptr inbounds [4 x float], [4 x float]* [[tmp]], i32 0, i32 {{.*}}
// OPTIMIZED: [[Result:%.+]] = load float, float* [[elAddr]], align 4

// OPTIMIZED: call void @dx.op.storeOutput.f32(i32 5, i32 0, i32 0, i8 0, float [[Result]])
// OPTIMIZED: call void @dx.op.storeOutput.f32(i32 5, i32 0, i32 0, i8 1, float [[Result]])
// OPTIMIZED: call void @dx.op.storeOutput.f32(i32 5, i32 0, i32 0, i8 2, float [[Result]])
// OPTIMIZED: call void @dx.op.storeOutput.f32(i32 5, i32 0, i32 0, i8 3, float [[Result]])
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// RUN: %dxc -T ps_6_0 -ast-dump-implicit %s | FileCheck %s

RWTexture1D<float4> Tex;

float4 main(uint i:I) : SV_Target {
return Tex[i];
}


// Verify the template construction, and the underlying record.
// CHECK: ClassTemplateDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> implicit RWTexture1D
// CHECK: CXXRecordDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> implicit class RWTexture1D definition
// CHECK-NEXT: FinalAttr {{0x[0-9a-fA-F]+}} <<invalid sloc>> Implicit final
// CHECK-NEXT: FieldDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> implicit h 'element'

// Make sure we don't process into another record decl.
// CHECK-NOT: CXXRecordDecl

// Verify the FunctionTemplateDecl for the subscript operator and the method
// containing the appropriate atributes.
// CHECK: FunctionTemplateDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> operator[]
// CHECK-NEXT: TemplateTypeParmDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> class element
// CHECK-NEXT: CXXMethodDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> operator[] 'element &(unsigned int) const'
// CHECK-NEXT: ParmVarDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> index 'unsigned int'
// CHECK-NEXT: HLSLCXXOverloadAttr {{0x[0-9a-fA-F]+}} <<invalid sloc>> Implicit
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// RUN: %dxc -T lib_6_4 -HV 2021 -ast-dump %s | FileCheck %s

struct MyVector3 {
float3 V;

float operator[](int x) {
return V[x];
}
};

float SomeFun(const MyVector3 V, int Idx) {
return V[Idx];
}

// This test verifies that under HLSL 2021, overload resolution for user-defined
// subscript operators follows HLSL overload resolution rules and ignores the
// const-ness of the implicit object parameter.

// CHECK: CXXRecordDecl {{0x[0-9a-fA-F]+}} {{.*}} referenced struct MyVector3 definition
// CHECK: FieldDecl {{0x[0-9a-fA-F]+}} <line:4:5, col:12> col:12 referenced V 'float3':'vector<float, 3>'
// CHECK-NEXT: CXXMethodDecl [[Operator:0x[0-9a-fA-F]+]] <line:6:5, line:8:5> line:6:11 used operator[] 'float (int)'

// CHECK: FunctionDecl {{0x[0-9a-fA-F]+}} <line:11:1, line:13:1> line:11:7 SomeFun 'float (const MyVector3, int)'
// CHECK: CXXOperatorCallExpr {{0x[0-9a-fA-F]+}} <col:10, col:15> 'float'
// CHECK-NEXT: ImplicitCastExpr {{0x[0-9a-fA-F]+}} <col:11, col:15> 'float (*)(int)' <FunctionToPointerDecay>
// CHECK-NEXT: DeclRefExpr {{0x[0-9a-fA-F]+}} <col:11, col:15> 'float (int)' lvalue CXXMethod [[Operator]] 'operator[]' 'float (int)'
29 changes: 29 additions & 0 deletions tools/clang/test/HLSLFileCheck/hlsl/types/matrix/matrix-ast.hlsl
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// RUN: %dxc -T lib_6_x -ast-dump-implicit %s | FileCheck %s

// Verify the matrix template formtaion: matrix< T = float, rows = 4, cols = 4>
// CHECK: ClassTemplateDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> implicit matrix
// CHECK-NEXT: TemplateTypeParmDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> class element
// CHECK-NEXT: TemplateArgument type 'float'
// CHECK-NEXT: NonTypeTemplateParmDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> 'int' row_count
// CHECK-NEXT: TemplateArgument expr
// CHECK-NEXT: IntegerLiteral {{0x[0-9a-fA-F]+}} <<invalid sloc>> 'int' 4
// CHECK-NEXT: NonTypeTemplateParmDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> 'int' col_count
// CHECK-NEXT: TemplateArgument expr
// CHECK-NEXT: IntegerLiteral {{0x[0-9a-fA-F]+}} <<invalid sloc>> 'int' 4

// Verify the record with the final attribute and the matrix field as an
// ext_vector array.
// CHECK-NEXT: CXXRecordDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> implicit class matrix definition
// CHECK-NEXT: FinalAttr {{0x[0-9a-fA-F]+}} <<invalid sloc>> Implicit final
// CHECK-NEXT: FieldDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> implicit h 'element [row_count] __attribute__((ext_vector_type(col_count)))'


// Verify non-const subscript operator overload.
// CHECK-NEXT: CXXMethodDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> operator[] 'vector<element, col_count> &(unsigned int)'
// CHECK-NEXT: ParmVarDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> index 'unsigned int'
// CHECK-NEXT: HLSLCXXOverloadAttr {{0x[0-9a-fA-F]+}} <<invalid sloc>> Implicit

// Verify const subscript operator overload.
// CHECK-NEXT: CXXMethodDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> operator[] 'vector<element, col_count> &const (unsigned int) const'
// CHECK-NEXT: ParmVarDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> index 'unsigned int'
// CHECK-NEXT: HLSLCXXOverloadAttr {{0x[0-9a-fA-F]+}} <<invalid sloc>> Implicit
34 changes: 34 additions & 0 deletions tools/clang/test/HLSLFileCheck/hlsl/types/vector/vector-ast.hlsl
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// RUN: %dxc -T lib_6_x -ast-dump-implicit %s | FileCheck %s

// Verify template and default arguments: vector<T = float, int = 4>

// CHECK: ClassTemplateDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> implicit vector
// CHECK-NEXT: TemplateTypeParmDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> class element
// CHECK-NEXT: TemplateArgument type 'float'
// CHECK-NEXT: NonTypeTemplateParmDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> 'int' element_count
// CHECK-NEXT: TemplateArgument expr
// CHECK-NEXT: IntegerLiteral {{0x[0-9a-fA-F]+}} <<invalid sloc>> 'int' 4

// Verify the class, final attribute and ext_vector field decl.
// CHECK-NEXT: CXXRecordDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> implicit class vector definition
// CHECK-NEXT: FinalAttr {{0x[0-9a-fA-F]+}} <<invalid sloc>> Implicit final
// CHECK-NEXT: FieldDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> implicit h 'element __attribute__((ext_vector_type(element_count)))'

// Verify operator overloads for const vector subscript operators.
// CHECK-NEXT: CXXMethodDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> operator[] 'element &const (unsigned int) const'
// CHECK-NEXT: ParmVarDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> index 'unsigned int'
// CHECK-NEXT: HLSLIntrinsicAttr {{0x[0-9a-fA-F]+}} <<invalid sloc>> Implicit "subscript" "" 7
// CHECK-NEXT: HLSLCXXOverloadAttr {{0x[0-9a-fA-F]+}} <<invalid sloc>> Implicit

// Verify operator overloads for non-const vector subscript operators.
// CHECK-NEXT: CXXMethodDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> operator[] 'element &(unsigned int)'
// CHECK-NEXT: ParmVarDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> index 'unsigned int'
// CHECK-NEXT: HLSLIntrinsicAttr {{0x[0-9a-fA-F]+}} <<invalid sloc>> Implicit "subscript" "" 7
// CHECK-NEXT: HLSLCXXOverloadAttr {{0x[0-9a-fA-F]+}} <<invalid sloc>> Implicit

// DXC forces specializations to exist for common types. This verifies the
// specializtion for float4 which comes next in the chain.
// CHECK-NEXT: ClassTemplateSpecializationDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> implicit class vector definition
// CHECK-NEXT: TemplateArgument type 'float'
// CHECK-NEXT: TemplateArgument integral 4

0 comments on commit aea5a49

Please sign in to comment.