[PATCH] D131268: [HLSL] Generate buffer subscript operators
This revision was automatically updated to reflect the committed changes.
Closed by commit rG00ecacca7d90: [HLSL] Generate buffer subscript operators
(authored by beanz).
Changed prior to commit:
https://reviews.llvm.org/D131268?vs=456774=457681#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131268/new/
https://reviews.llvm.org/D131268
Files:
clang/lib/Sema/HLSLExternalSemaSource.cpp
clang/lib/Sema/SemaType.cpp
clang/test/AST/HLSL/RWBuffer-AST.hlsl
clang/test/CodeGenHLSL/buffer-array-operator.hlsl
Index: clang/test/CodeGenHLSL/buffer-array-operator.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/buffer-array-operator.hlsl
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s
+
+const RWBuffer In;
+RWBuffer Out;
+
+void fn(int Idx) {
+ Out[Idx] = In[Idx];
+}
+
+// This test is intended to verify reasonable code generation of the subscript
+// operator. In this test case we should be generating both the const and
+// non-const operators so we verify both cases.
+
+// Non-const comes first.
+// CHECK: ptr @"??A?$RWBuffer@M@hlsl@@QBAAAMI@Z"
+// CHECK: %this1 = load ptr, ptr %this.addr, align 4
+// CHECK-NEXT: %h = getelementptr inbounds %"class.hlsl::RWBuffer", ptr %this1, i32 0, i32 0
+// CHECK-NEXT: %0 = load ptr, ptr %h, align 4
+// CHECK-NEXT: %1 = load i32, ptr %Idx.addr, align 4
+// CHECK-NEXT: %arrayidx = getelementptr inbounds float, ptr %0, i32 %1
+// CHECK-NEXT: ret ptr %arrayidx
+
+// Const comes next, and returns the pointer instead of the value.
+// CHECK: ptr @"??A?$RWBuffer@M@hlsl@@QMI@Z"
+// CHECK: %this1 = load ptr, ptr %this.addr, align 4
+// CHECK-NEXT: %h = getelementptr inbounds %"class.hlsl::RWBuffer", ptr %this1, i32 0, i32 0
+// CHECK-NEXT: %0 = load ptr, ptr %h, align 4
+// CHECK-NEXT: %1 = load i32, ptr %Idx.addr, align 4
+// CHECK-NEXT: %arrayidx = getelementptr inbounds float, ptr %0, i32 %1
+// CHECK-NEXT: ret ptr %arrayidx
Index: clang/test/AST/HLSL/RWBuffer-AST.hlsl
===
--- clang/test/AST/HLSL/RWBuffer-AST.hlsl
+++ clang/test/AST/HLSL/RWBuffer-AST.hlsl
@@ -39,11 +39,30 @@
// CHECK: FinalAttr 0x{{[0-9A-Fa-f]+}} <> Implicit final
// CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <> Implicit UAV
-// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <> implicit h 'void *'
+// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <> implicit h 'element_type *'
+
+// CHECK: CXXMethodDecl 0x{{[0-9A-Fa-f]+}} <> operator[] 'element_type (unsigned int) const'
+// CHECK-NEXT: ParmVarDecl 0x{{[0-9A-Fa-f]+}} <> Idx 'unsigned int'
+// CHECK-NEXT: CompoundStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ReturnStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ArraySubscriptExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type' lvalue
+// CHECK-NEXT: MemberExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type *' lvalue ->h 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT: CXXThisExpr 0x{{[0-9A-Fa-f]+}} <> 'const RWBuffer *' implicit this
+// CHECK-NEXT: DeclRefExpr 0x{{[0-9A-Fa-f]+}} <> 'unsigned int' ParmVar 0x{{[0-9A-Fa-f]+}} 'Idx' 'unsigned int'
+
+// CHECK-NEXT: CXXMethodDecl 0x{{[0-9A-Fa-f]+}} <> operator[] 'element_type &(unsigned int)'
+// CHECK-NEXT: ParmVarDecl 0x{{[0-9A-Fa-f]+}} <> Idx 'unsigned int'
+// CHECK-NEXT: CompoundStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ReturnStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ArraySubscriptExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type' lvalue
+// CHECK-NEXT: MemberExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type *' lvalue ->h 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT: CXXThisExpr 0x{{[0-9A-Fa-f]+}} <> 'RWBuffer *' implicit this
+// CHECK-NEXT: DeclRefExpr 0x{{[0-9A-Fa-f]+}} <> 'unsigned int' ParmVar 0x{{[0-9A-Fa-f]+}} 'Idx' 'unsigned int'
+
// CHECK: ClassTemplateSpecializationDecl 0x{{[0-9A-Fa-f]+}} <> class RWBuffer definition
// CHECK: TemplateArgument type 'float'
// CHECK-NEXT: BuiltinType 0x{{[0-9A-Fa-f]+}} 'float'
// CHECK-NEXT: FinalAttr 0x{{[0-9A-Fa-f]+}} <> Implicit final
// CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <> Implicit UAV
-// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <> implicit referenced h 'void *'
+// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <> implicit referenced h 'float *'
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2174,7 +2174,7 @@
return QualType();
}
- if (getLangOpts().HLSL) {
+ if (getLangOpts().HLSL && Loc.isValid()) {
Diag(Loc, diag::err_hlsl_pointers_unsupported) << 0;
return QualType();
}
@@ -2244,7 +2244,7 @@
return QualType();
}
- if (getLangOpts().HLSL) {
+ if (getLangOpts().HLSL && Loc.isValid()) {
Diag(Loc, diag::err_hlsl_pointers_unsupported) << 1;
return QualType();
}
@@ -3008,7 +3008,7 @@
return QualType();
}
[PATCH] D131268: [HLSL] Generate buffer subscript operators
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM aside from a nit that was missed. Thanks for switching to static_cast,
that makes me happier. :-)
Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:109
+if (Template) {
+ if (const auto TTD = dyn_cast(
+ Template->getTemplateParameters()->getParam(0)))
Missed from the previous suggestion.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131268/new/
https://reviews.llvm.org/D131268
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D131268: [HLSL] Generate buffer subscript operators
beanz updated this revision to Diff 456774.
beanz added a comment.
Updating based on @aaron.ballman's feedback.
- Change reinterpret_cast -> static_cast
- Aaron likes `auto` more than me... but all in good places :D
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131268/new/
https://reviews.llvm.org/D131268
Files:
clang/lib/Sema/HLSLExternalSemaSource.cpp
clang/lib/Sema/SemaType.cpp
clang/test/AST/HLSL/RWBuffer-AST.hlsl
clang/test/CodeGenHLSL/buffer-array-operator.hlsl
Index: clang/test/CodeGenHLSL/buffer-array-operator.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/buffer-array-operator.hlsl
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s
+
+const RWBuffer In;
+RWBuffer Out;
+
+void fn(int Idx) {
+ Out[Idx] = In[Idx];
+}
+
+// This test is intended to verify reasonable code generation of the subscript
+// operator. In this test case we should be generating both the const and
+// non-const operators so we verify both cases.
+
+// Non-const comes first.
+// CHECK: ptr @"??A?$RWBuffer@M@hlsl@@QBAAAMI@Z"
+// CHECK: %this1 = load ptr, ptr %this.addr, align 4
+// CHECK-NEXT: %h = getelementptr inbounds %"class.hlsl::RWBuffer", ptr %this1, i32 0, i32 0
+// CHECK-NEXT: %0 = load ptr, ptr %h, align 4
+// CHECK-NEXT: %1 = load i32, ptr %Idx.addr, align 4
+// CHECK-NEXT: %arrayidx = getelementptr inbounds float, ptr %0, i32 %1
+// CHECK-NEXT: ret ptr %arrayidx
+
+// Const comes next, and returns the pointer instead of the value.
+// CHECK: ptr @"??A?$RWBuffer@M@hlsl@@QMI@Z"
+// CHECK: %this1 = load ptr, ptr %this.addr, align 4
+// CHECK-NEXT: %h = getelementptr inbounds %"class.hlsl::RWBuffer", ptr %this1, i32 0, i32 0
+// CHECK-NEXT: %0 = load ptr, ptr %h, align 4
+// CHECK-NEXT: %1 = load i32, ptr %Idx.addr, align 4
+// CHECK-NEXT: %arrayidx = getelementptr inbounds float, ptr %0, i32 %1
+// CHECK-NEXT: ret ptr %arrayidx
Index: clang/test/AST/HLSL/RWBuffer-AST.hlsl
===
--- clang/test/AST/HLSL/RWBuffer-AST.hlsl
+++ clang/test/AST/HLSL/RWBuffer-AST.hlsl
@@ -39,11 +39,30 @@
// CHECK: FinalAttr 0x{{[0-9A-Fa-f]+}} <> Implicit final
// CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <> Implicit UAV
-// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <> implicit h 'void *'
+// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <> implicit h 'element_type *'
+
+// CHECK: CXXMethodDecl 0x{{[0-9A-Fa-f]+}} <> operator[] 'element_type (unsigned int) const'
+// CHECK-NEXT: ParmVarDecl 0x{{[0-9A-Fa-f]+}} <> Idx 'unsigned int'
+// CHECK-NEXT: CompoundStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ReturnStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ArraySubscriptExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type' lvalue
+// CHECK-NEXT: MemberExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type *' lvalue ->h 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT: CXXThisExpr 0x{{[0-9A-Fa-f]+}} <> 'const RWBuffer *' implicit this
+// CHECK-NEXT: DeclRefExpr 0x{{[0-9A-Fa-f]+}} <> 'unsigned int' ParmVar 0x{{[0-9A-Fa-f]+}} 'Idx' 'unsigned int'
+
+// CHECK-NEXT: CXXMethodDecl 0x{{[0-9A-Fa-f]+}} <> operator[] 'element_type &(unsigned int)'
+// CHECK-NEXT: ParmVarDecl 0x{{[0-9A-Fa-f]+}} <> Idx 'unsigned int'
+// CHECK-NEXT: CompoundStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ReturnStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ArraySubscriptExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type' lvalue
+// CHECK-NEXT: MemberExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type *' lvalue ->h 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT: CXXThisExpr 0x{{[0-9A-Fa-f]+}} <> 'RWBuffer *' implicit this
+// CHECK-NEXT: DeclRefExpr 0x{{[0-9A-Fa-f]+}} <> 'unsigned int' ParmVar 0x{{[0-9A-Fa-f]+}} 'Idx' 'unsigned int'
+
// CHECK: ClassTemplateSpecializationDecl 0x{{[0-9A-Fa-f]+}} <> class RWBuffer definition
// CHECK: TemplateArgument type 'float'
// CHECK-NEXT: BuiltinType 0x{{[0-9A-Fa-f]+}} 'float'
// CHECK-NEXT: FinalAttr 0x{{[0-9A-Fa-f]+}} <> Implicit final
// CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <> Implicit UAV
-// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <> implicit referenced h 'void *'
+// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <> implicit referenced h 'float *'
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2174,7 +2174,7 @@
return QualType();
}
- if (getLangOpts().HLSL) {
+ if (getLangOpts().HLSL && Loc.isValid()) {
Diag(Loc, diag::err_hlsl_pointers_unsupported) << 0;
return QualType();
}
@@ -2244,7 +2244,7 @@
return QualType();
}
- if (getLangOpts().HLSL) {
+ if (getLangOpts().HLSL && Loc.isValid()) {
Diag(Loc, diag::err_hlsl_pointers_unsupported) << 1;
return QualType();
}
@@ -3008,7 +3008,7 @@
return QualType();
}
- if (getLangOpts().HLSL) {
+
[PATCH] D131268: [HLSL] Generate buffer subscript operators
beanz added inline comments. Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:243 +AST, MethodDecl->getDeclContext(), SourceLocation(), SourceLocation(), +, AST.UnsignedIntTy, +AST.getTrivialTypeSourceInfo(AST.UnsignedIntTy, SourceLocation()), aaron.ballman wrote: > Should this be using a size type instead? As currently implemented it is an unsigned int. I should include the doc link in the description: https://docs.microsoft.com/en-us/windows/win32/direct3dhlsl/sm5-object-rwbuffer-operatorindex Eventually I think we'll need to introduce size types to HLSL, but we currently don't really use them. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131268/new/ https://reviews.llvm.org/D131268 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D131268: [HLSL] Generate buffer subscript operators
aaron.ballman added inline comments.
Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:108-113
+if (Template) {
+ if (auto TTD = dyn_cast(
+ Template->getTemplateParameters()->getParam(0)))
+Ty = Record->getASTContext().getPointerType(
+QualType(TTD->getTypeForDecl(), 0));
+}
Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:180
+if (Handle->getType().getCanonicalType() != AST.VoidPtrTy) {
+ Call = CXXReinterpretCastExpr::Create(
+ AST, Handle->getType(), VK_PRValue, CK_Dependent, Call, nullptr,
Can you use a `static_cast` to make this a wee bit safer, or is that not
possible?
Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:231-232
+AST.getFunctionType(ReturnTy, {AST.UnsignedIntTy}, ExtInfo);
+auto TSInfo = AST.getTrivialTypeSourceInfo(MethodTy, SourceLocation());
+auto MethodDecl = CXXMethodDecl::Create(
+AST, Record, SourceLocation(),
Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:241
+IdentifierInfo = AST.Idents.get("Idx", tok::TokenKind::identifier);
+auto IdxParam = ParmVarDecl::Create(
+AST, MethodDecl->getDeclContext(), SourceLocation(), SourceLocation(),
Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:243
+AST, MethodDecl->getDeclContext(), SourceLocation(), SourceLocation(),
+, AST.UnsignedIntTy,
+AST.getTrivialTypeSourceInfo(AST.UnsignedIntTy, SourceLocation()),
Should this be using a size type instead?
Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:252
+
+CXXThisExpr *This = new (AST)
+CXXThisExpr(SourceLocation(), MethodDecl->getThisType(), true);
Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:254
+CXXThisExpr(SourceLocation(), MethodDecl->getThisType(), true);
+Expr *HandleAccess = MemberExpr::CreateImplicit(
+AST, This, true, Handle, Handle->getType(), VK_LValue, OK_Ordinary);
Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:257
+
+Expr *IndexExpr = DeclRefExpr::Create(
+AST, NestedNameSpecifierLoc(), SourceLocation(), IdxParam, false,
Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:262
+
+Expr *Array =
+new (AST) ArraySubscriptExpr(HandleAccess, IndexExpr, ElemTy,
VK_LValue,
Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:266
+
+Stmt *Return = ReturnStmt::Create(AST, SourceLocation(), Array, nullptr);
+
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131268/new/
https://reviews.llvm.org/D131268
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D131268: [HLSL] Generate buffer subscript operators
beanz updated this revision to Diff 456731.
beanz added a comment.
Updating based on PR feedback and rebasing.
- Rebased on main today
- Made const subscript return type const &
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131268/new/
https://reviews.llvm.org/D131268
Files:
clang/lib/Sema/HLSLExternalSemaSource.cpp
clang/lib/Sema/SemaType.cpp
clang/test/AST/HLSL/RWBuffer-AST.hlsl
clang/test/CodeGenHLSL/buffer-array-operator.hlsl
Index: clang/test/CodeGenHLSL/buffer-array-operator.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/buffer-array-operator.hlsl
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s
+
+const RWBuffer In;
+RWBuffer Out;
+
+void fn(int Idx) {
+ Out[Idx] = In[Idx];
+}
+
+// This test is intended to verify reasonable code generation of the subscript
+// operator. In this test case we should be generating both the const and
+// non-const operators so we verify both cases.
+
+// Non-const comes first.
+// CHECK: ptr @"??A?$RWBuffer@M@hlsl@@QBAAAMI@Z"
+// CHECK: %this1 = load ptr, ptr %this.addr, align 4
+// CHECK-NEXT: %h = getelementptr inbounds %"class.hlsl::RWBuffer", ptr %this1, i32 0, i32 0
+// CHECK-NEXT: %0 = load ptr, ptr %h, align 4
+// CHECK-NEXT: %1 = load i32, ptr %Idx.addr, align 4
+// CHECK-NEXT: %arrayidx = getelementptr inbounds float, ptr %0, i32 %1
+// CHECK-NEXT: ret ptr %arrayidx
+
+// Const comes next, and returns the pointer instead of the value.
+// CHECK: ptr @"??A?$RWBuffer@M@hlsl@@QMI@Z"
+// CHECK: %this1 = load ptr, ptr %this.addr, align 4
+// CHECK-NEXT: %h = getelementptr inbounds %"class.hlsl::RWBuffer", ptr %this1, i32 0, i32 0
+// CHECK-NEXT: %0 = load ptr, ptr %h, align 4
+// CHECK-NEXT: %1 = load i32, ptr %Idx.addr, align 4
+// CHECK-NEXT: %arrayidx = getelementptr inbounds float, ptr %0, i32 %1
+// CHECK-NEXT: ret ptr %arrayidx
Index: clang/test/AST/HLSL/RWBuffer-AST.hlsl
===
--- clang/test/AST/HLSL/RWBuffer-AST.hlsl
+++ clang/test/AST/HLSL/RWBuffer-AST.hlsl
@@ -39,11 +39,30 @@
// CHECK: FinalAttr 0x{{[0-9A-Fa-f]+}} <> Implicit final
// CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <> Implicit UAV
-// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <> implicit h 'void *'
+// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <> implicit h 'element_type *'
+
+// CHECK: CXXMethodDecl 0x{{[0-9A-Fa-f]+}} <> operator[] 'element_type (unsigned int) const'
+// CHECK-NEXT: ParmVarDecl 0x{{[0-9A-Fa-f]+}} <> Idx 'unsigned int'
+// CHECK-NEXT: CompoundStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ReturnStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ArraySubscriptExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type' lvalue
+// CHECK-NEXT: MemberExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type *' lvalue ->h 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT: CXXThisExpr 0x{{[0-9A-Fa-f]+}} <> 'const RWBuffer *' implicit this
+// CHECK-NEXT: DeclRefExpr 0x{{[0-9A-Fa-f]+}} <> 'unsigned int' ParmVar 0x{{[0-9A-Fa-f]+}} 'Idx' 'unsigned int'
+
+// CHECK-NEXT: CXXMethodDecl 0x{{[0-9A-Fa-f]+}} <> operator[] 'element_type &(unsigned int)'
+// CHECK-NEXT: ParmVarDecl 0x{{[0-9A-Fa-f]+}} <> Idx 'unsigned int'
+// CHECK-NEXT: CompoundStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ReturnStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ArraySubscriptExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type' lvalue
+// CHECK-NEXT: MemberExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type *' lvalue ->h 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT: CXXThisExpr 0x{{[0-9A-Fa-f]+}} <> 'RWBuffer *' implicit this
+// CHECK-NEXT: DeclRefExpr 0x{{[0-9A-Fa-f]+}} <> 'unsigned int' ParmVar 0x{{[0-9A-Fa-f]+}} 'Idx' 'unsigned int'
+
// CHECK: ClassTemplateSpecializationDecl 0x{{[0-9A-Fa-f]+}} <> class RWBuffer definition
// CHECK: TemplateArgument type 'float'
// CHECK-NEXT: BuiltinType 0x{{[0-9A-Fa-f]+}} 'float'
// CHECK-NEXT: FinalAttr 0x{{[0-9A-Fa-f]+}} <> Implicit final
// CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <> Implicit UAV
-// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <> implicit referenced h 'void *'
+// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <> implicit referenced h 'float *'
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2174,7 +2174,7 @@
return QualType();
}
- if (getLangOpts().HLSL) {
+ if (getLangOpts().HLSL && Loc.isValid()) {
Diag(Loc, diag::err_hlsl_pointers_unsupported) << 0;
return QualType();
}
@@ -2244,7 +2244,7 @@
return QualType();
}
- if (getLangOpts().HLSL) {
+ if (getLangOpts().HLSL && Loc.isValid()) {
Diag(Loc, diag::err_hlsl_pointers_unsupported) << 1;
return QualType();
}
@@ -3008,7 +3008,7 @@
return QualType();
}
- if (getLangOpts().HLSL) {
+ if (getLangOpts().HLSL &&
[PATCH] D131268: [HLSL] Generate buffer subscript operators
beanz added inline comments. Comment at: clang/test/CodeGenHLSL/buffer-array-operator.hlsl:3 + +const RWBuffer In; +RWBuffer Out; python3kgae wrote: > beanz wrote: > > python3kgae wrote: > > > Why add const instead of using Buffer directly? > > > > > Making this const forces the const methods to be used. It is just to drive > > the correct validation and code generation. > So maybe we don't need Buffer at all, just use const RWBuffer for read-only > usage? I think there would likely be some code incompatibilities if we alias `Buffer` to `const RWBuffer`, but that would be interesting to consider. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131268/new/ https://reviews.llvm.org/D131268 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D131268: [HLSL] Generate buffer subscript operators
python3kgae added inline comments. Comment at: clang/test/CodeGenHLSL/buffer-array-operator.hlsl:3 + +const RWBuffer In; +RWBuffer Out; beanz wrote: > python3kgae wrote: > > Why add const instead of using Buffer directly? > > > Making this const forces the const methods to be used. It is just to drive > the correct validation and code generation. So maybe we don't need Buffer at all, just use const RWBuffer for read-only usage? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131268/new/ https://reviews.llvm.org/D131268 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D131268: [HLSL] Generate buffer subscript operators
beanz added inline comments. Comment at: clang/test/CodeGenHLSL/buffer-array-operator.hlsl:3 + +const RWBuffer In; +RWBuffer Out; python3kgae wrote: > Why add const instead of using Buffer directly? > Making this const forces the const methods to be used. It is just to drive the correct validation and code generation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131268/new/ https://reviews.llvm.org/D131268 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D131268: [HLSL] Generate buffer subscript operators
python3kgae added inline comments. Comment at: clang/test/CodeGenHLSL/buffer-array-operator.hlsl:3 + +const RWBuffer In; +RWBuffer Out; Why add const instead of using Buffer directly? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131268/new/ https://reviews.llvm.org/D131268 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D131268: [HLSL] Generate buffer subscript operators
beanz added inline comments. Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:220 + +// Const subscript operators return copies of elements, non-const return a +// reference so that they are assignable. python3kgae wrote: > If we reuse this function for StructuredBuffer, then const subscript return a > const reference could be better? Why? Since this should get inlined return value optimization should eliminate the redundant copies. Comment at: clang/test/CodeGenHLSL/buffer-array-operator.hlsl:4 +const RWBuffer In; +RWBuffer Out; + python3kgae wrote: > Maybe change In to RWBuffer > And Out[Idx] = In[Idx].z; > So we also test vector case? None of the added code here depends on the template parameter type. Putting a vector in increases the complexity of the output, but I'm unsure it extends the test coverage meaningfully. Comment at: clang/test/CodeGenHLSL/buffer-array-operator.hlsl:16 +// CHECK: float @"??A?$RWBuffer@M@hlsl@@QBAMI@Z" +// CHECK: %this1 = load ptr, ptr %this.addr, align 4 +// CHECK-NEXT: %h = getelementptr inbounds %"class.hlsl::RWBuffer", ptr %this1, i32 0, i32 0 python3kgae wrote: > Where is the this.addr coming from? That is how clang generates stored locations for input parameters. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131268/new/ https://reviews.llvm.org/D131268 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D131268: [HLSL] Generate buffer subscript operators
python3kgae added inline comments. Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:220 + +// Const subscript operators return copies of elements, non-const return a +// reference so that they are assignable. If we reuse this function for StructuredBuffer, then const subscript return a const reference could be better? Comment at: clang/test/CodeGenHLSL/buffer-array-operator.hlsl:4 +const RWBuffer In; +RWBuffer Out; + Maybe change In to RWBuffer And Out[Idx] = In[Idx].z; So we also test vector case? Comment at: clang/test/CodeGenHLSL/buffer-array-operator.hlsl:16 +// CHECK: float @"??A?$RWBuffer@M@hlsl@@QBAMI@Z" +// CHECK: %this1 = load ptr, ptr %this.addr, align 4 +// CHECK-NEXT: %h = getelementptr inbounds %"class.hlsl::RWBuffer", ptr %this1, i32 0, i32 0 Where is the this.addr coming from? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131268/new/ https://reviews.llvm.org/D131268 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D131268: [HLSL] Generate buffer subscript operators
beanz created this revision.
beanz added reviewers: bogner, aaron.ballman, python3kgae, pow2clk.
Herald added a subscriber: Anastasia.
Herald added a project: All.
beanz requested review of this revision.
Herald added a project: clang.
In HLSL buffer types support array subscripting syntax for loads and
stores. This change fleshes out the subscript operators to become array
accesses on the underlying handle pointer. This will allow LLVM
optimization passes to optimize resource accesses the same way any other
memory access would be optimized.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D131268
Files:
clang/lib/Sema/HLSLExternalSemaSource.cpp
clang/lib/Sema/SemaType.cpp
clang/test/AST/HLSL/RWBuffer-AST.hlsl
clang/test/CodeGenHLSL/buffer-array-operator.hlsl
Index: clang/test/CodeGenHLSL/buffer-array-operator.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/buffer-array-operator.hlsl
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s
+
+const RWBuffer In;
+RWBuffer Out;
+
+void fn(int Idx) {
+ Out[Idx] = In[Idx];
+}
+
+// This test is intended to verify reasonable code generation of the subscript
+// operator. In this test case we should be generating both the const and
+// non-const operators so we verify both cases.
+
+// Non-const comes first.
+// CHECK: float @"??A?$RWBuffer@M@hlsl@@QBAMI@Z"
+// CHECK: %this1 = load ptr, ptr %this.addr, align 4
+// CHECK-NEXT: %h = getelementptr inbounds %"class.hlsl::RWBuffer", ptr %this1, i32 0, i32 0
+// CHECK-NEXT: %0 = load ptr, ptr %h, align 4
+// CHECK-NEXT: %1 = load i32, ptr %Idx.addr, align 4
+// CHECK-NEXT: %arrayidx = getelementptr inbounds float, ptr %0, i32 %1
+// CHECK-NEXT: %2 = load float, ptr %arrayidx, align 4
+// CHECK-NEXT: ret float %2
+
+// Const comes next, and returns the pointer instead of the value.
+// CHECK: ptr @"??A?$RWBuffer@M@hlsl@@QMI@Z"
+// CHECK: %this1 = load ptr, ptr %this.addr, align 4
+// CHECK-NEXT: %h = getelementptr inbounds %"class.hlsl::RWBuffer", ptr %this1, i32 0, i32 0
+// CHECK-NEXT: %0 = load ptr, ptr %h, align 4
+// CHECK-NEXT: %1 = load i32, ptr %Idx.addr, align 4
+// CHECK-NEXT: %arrayidx = getelementptr inbounds float, ptr %0, i32 %1
+// CHECK-NEXT: ret ptr %arrayidx
\ No newline at end of file
Index: clang/test/AST/HLSL/RWBuffer-AST.hlsl
===
--- clang/test/AST/HLSL/RWBuffer-AST.hlsl
+++ clang/test/AST/HLSL/RWBuffer-AST.hlsl
@@ -39,11 +39,30 @@
// CHECK: FinalAttr 0x{{[0-9A-Fa-f]+}} <> Implicit final
// CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <> Implicit UAV
-// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <> implicit h 'void *'
+// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <> implicit h 'element_type *'
+
+// CHECK: CXXMethodDecl 0x{{[0-9A-Fa-f]+}} <> operator[] 'element_type (unsigned int) const'
+// CHECK-NEXT: ParmVarDecl 0x{{[0-9A-Fa-f]+}} <> Idx 'unsigned int'
+// CHECK-NEXT: CompoundStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ReturnStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ArraySubscriptExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type' lvalue
+// CHECK-NEXT: MemberExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type *' lvalue ->h 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT: CXXThisExpr 0x{{[0-9A-Fa-f]+}} <> 'const RWBuffer *' implicit this
+// CHECK-NEXT: DeclRefExpr 0x{{[0-9A-Fa-f]+}} <> 'unsigned int' ParmVar 0x{{[0-9A-Fa-f]+}} 'Idx' 'unsigned int'
+
+// CHECK-NEXT: CXXMethodDecl 0x{{[0-9A-Fa-f]+}} <> operator[] 'element_type &(unsigned int)'
+// CHECK-NEXT: ParmVarDecl 0x{{[0-9A-Fa-f]+}} <> Idx 'unsigned int'
+// CHECK-NEXT: CompoundStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ReturnStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ArraySubscriptExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type' lvalue
+// CHECK-NEXT: MemberExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type *' lvalue ->h 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT: CXXThisExpr 0x{{[0-9A-Fa-f]+}} <> 'RWBuffer *' implicit this
+// CHECK-NEXT: DeclRefExpr 0x{{[0-9A-Fa-f]+}} <> 'unsigned int' ParmVar 0x{{[0-9A-Fa-f]+}} 'Idx' 'unsigned int'
+
// CHECK: ClassTemplateSpecializationDecl 0x{{[0-9A-Fa-f]+}} <> class RWBuffer definition
// CHECK: TemplateArgument type 'float'
// CHECK-NEXT: BuiltinType 0x{{[0-9A-Fa-f]+}} 'float'
// CHECK-NEXT: FinalAttr 0x{{[0-9A-Fa-f]+}} <> Implicit final
// CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <> Implicit UAV
-// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <> implicit referenced h 'void *'
+// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <> implicit referenced h 'float *'
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2158,7 +2158,7 @@
return QualType();
}
- if (getLangOpts().HLSL) {
+ if (getLangOpts().HLSL && Loc.isValid()) {
Diag(Loc,
