[PATCH] D135721: [HLSL] Added HLSL this as a reference
gracejennings marked 5 inline comments as done.
gracejennings added inline comments.
Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:179
+AST, SourceLocation(),
+Constructor->getThisType().getTypePtr()->getPointeeType(), true);
+This->setValueKind(ExprValueKind::VK_LValue);
python3kgae wrote:
> gracejennings wrote:
> > python3kgae wrote:
> > > gracejennings wrote:
> > > > python3kgae wrote:
> > > > > Should this be a reference type?
> > > > Could you expand on the question? I'm not sure I understand what you're
> > > > asking. The two changes in this file were made to update the previous
> > > > RWBuffer implementation
> > > The current code will create CXXThisExpr with the pointeeType.
> > > I thought it should be a reference type of the pointeeType.
> > >
> > > Like in the test,
> > > CXXThisExpr 0x{{[0-9A-Fa-f]+}} <> 'RWBuffer
> > > *' implicit this
> > >
> > > The type is RWBuffer * before,
> > > I expected this patch will change it to
> > > RWBuffer &.
> > The change that makes it more reference like than c++ from:
> >
> > `-MemberExpr 0x{{[0-9A-Fa-f]+}} 'int' lvalue ->First
> > 0x{{[0-9A-Fa-f]+}}`
> > `-CXXThisExpr 0x{{[0-9A-Fa-f]+}} 'Pair *' this`
> >
> > to hlsl with this change
> >
> > `-MemberExpr 0x{{[0-9A-Fa-f]+}} 'int' lvalue .First
> > 0x{{[0-9A-Fa-f]+}}`
> > `-CXXThisExpr 0x{{[0-9A-Fa-f]+}} 'Pair' lvalue this`
> I guess we have to change clang codeGen for this anyway.
>
> Not sure which has less impact for codeGen side, lvalue like what is in the
> current patch or make it a lvalue reference? My feeling is lvalue reference
> might be eaiser.
>
> Did you test what needs to change for clang codeGen on top of the current
> patch?
>
With just the lvalue change in the current patch there should be no additional
changes needed in clang CodeGen on top of the current patch
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135721/new/
https://reviews.llvm.org/D135721
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D135721: [HLSL] Added HLSL this as a reference
gracejennings created this revision.
gracejennings added reviewers: python3kgae, beanz, pow2clk, bob80905.
Herald added a subscriber: Anastasia.
Herald added a project: All.
gracejennings requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
This change makes `this` a reference instead of a pointer in
HLSL. HLSL does not have the `->` operator, and accesses through `this`
are with the `.` syntax. Tests were added and altered to make sure
the AST accurately reflects the types.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D135721
Files:
clang/include/clang/AST/ExprCXX.h
clang/lib/AST/ExprClassification.cpp
clang/lib/CodeGen/CGExpr.cpp
clang/lib/Sema/HLSLExternalSemaSource.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/lib/Sema/SemaExprMember.cpp
clang/test/AST/HLSL/RWBuffer-AST.hlsl
clang/test/AST/HLSL/this-reference-template.hlsl
clang/test/AST/HLSL/this-reference.hlsl
Index: clang/test/AST/HLSL/this-reference.hlsl
===
--- /dev/null
+++ clang/test/AST/HLSL/this-reference.hlsl
@@ -0,0 +1,62 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -disable-llvm-passes -o - -hlsl-entry main %s | FileCheck %s
+
+class Pair {
+ int First;
+ int Second;
+
+ int getFirst() {
+ return this.First;
+ }
+
+ int getSecond() {
+return Second;
+ }
+};
+
+class PairInfo : Pair {
+ int Sum;
+
+ int getSum() {
+return this.First + Second;
+ }
+};
+
+[numthreads(1, 1, 1)]
+void main() {
+ Pair Vals = {1, 2};
+ Vals.First = Vals.getFirst();
+ Vals.Second = Vals.getSecond();
+
+ PairInfo ValsInfo;
+ ValsInfo.First = Vals.First;
+ ValsInfo.Second = Vals.Second;
+ ValsInfo.Sum = ValsInfo.getSum();
+
+}
+
+// CHECK: -CXXMethodDecl 0x{{[0-9A-Fa-f]+}} line:7:7 used getFirst 'int ()' implicit-inline
+// CHECK-NEXT:`-CompoundStmt 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:`-ReturnStmt 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:`-ImplicitCastExpr 0x{{[0-9A-Fa-f]+}} 'int'
+// CHECK-NEXT:`-MemberExpr 0x{{[0-9A-Fa-f]+}} 'int' lvalue .First 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:`-CXXThisExpr 0x{{[0-9A-Fa-f]+}} 'Pair' lvalue this
+// CHECK-NEXT:-CXXMethodDecl 0x{{[0-9A-Fa-f]+}} line:11:7 used getSecond 'int ()' implicit-inline
+// CHECK-NEXT:`-CompoundStmt 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:`-ReturnStmt 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:`-ImplicitCastExpr 0x{{[0-9A-Fa-f]+}} 'int'
+// CHECK-NEXT:`-MemberExpr 0x{{[0-9A-Fa-f]+}} 'int' lvalue .Second 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:`-CXXThisExpr 0x{{[0-9A-Fa-f]+}} 'Pair' lvalue implicit this
+
+
+// CHECK: CXXMethodDecl 0x{{[0-9A-Fa-f]+}} line:19:7 used getSum 'int ()' implicit-inline
+// CHECK-NEXT:`-CompoundStmt 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:`-ReturnStmt 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:`-BinaryOperator 0x{{[0-9A-Fa-f]+}} 'int' '+'
+// CHECK-NEXT:-ImplicitCastExpr 0x{{[0-9A-Fa-f]+}} 'int'
+// CHECK-NEXT:`-MemberExpr 0x{{[0-9A-Fa-f]+}} 'int' lvalue .First 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:`-ImplicitCastExpr 0x{{[0-9A-Fa-f]+}} 'Pair' lvalue
+// CHECK-NEXT:`-CXXThisExpr 0x{{[0-9A-Fa-f]+}} 'PairInfo' lvalue this
+// CHECK-NEXT:`-ImplicitCastExpr 0x{{[0-9A-Fa-f]+}} 'int'
+// CHECK-NEXT:`-MemberExpr 0x{{[0-9A-Fa-f]+}} 'int' lvalue .Second 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:`-ImplicitCastExpr 0x{{[0-9A-Fa-f]+}} 'Pair' lvalue
+// CHECK-NEXT:`-CXXThisExpr 0x{{[0-9A-Fa-f]+}} 'PairInfo' lvalue implicit this
\ No newline at end of file
Index: clang/test/AST/HLSL/this-reference-template.hlsl
===
--- /dev/null
+++ clang/test/AST/HLSL/this-reference-template.hlsl
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -disable-llvm-passes -o - -hlsl-entry main %s | FileCheck %s
+
+template
+struct Pair {
+ K First;
+ V Second;
+
+ K getFirst() {
+ return this.First;
+ }
+
+ V getSecond() {
+return Second;
+ }
+};
+
+[numthreads(1, 1, 1)]
+void main() {
+ Pair Vals = {1, 2.0};
+ Vals.First = Vals.getFirst();
+ Vals.Second = Vals.getSecond();
+}
+
+// CHECK: -CXXMethodDecl 0x{{[0-9A-Fa-f]+}} line:8:5 getFirst 'K ()' implicit-inline
+// CHECK-NEXT:-CompoundStmt 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:-ReturnStmt 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:-CXXDependentScopeMemberExpr 0x{{[0-9A-Fa-f]+}} '' lvalue .First
+// CHECK-NEXT:-CXXThisExpr 0x{{[0-9A-Fa-f]+}} 'Pair' lvalue this
+// CHECK-NEXT:-CXXMethodDecl 0x{{[0-9A-Fa-f]+}} line:12:5 getSecond 'V ()' implicit-inline
+// CHECK-NEXT:-CompoundStmt 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:-ReturnStmt 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:-MemberExpr 0x{{[0-9A-Fa-f]+}} 'V' lvalue .Second 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:-CXXThisExpr 0x{{[0-9A-Fa-f]+}} 'Pair' lvalue implicit this
+
+//CHECK: -CXXMethodDecl 0x{{[0-9A-Fa-f]+}} line:8:5 used getFirst 'int ()' implicit-inline
+// CHECK-NEXT:-CompoundStmt 0x{{[0-9A-Fa-f]+}}
+// CHECK-
[PATCH] D135721: [HLSL] Added HLSL this as a reference
gracejennings updated this revision to Diff 466962.
gracejennings added a comment.
Formatting
Added new line file ending
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135721/new/
https://reviews.llvm.org/D135721
Files:
clang/test/AST/HLSL/this-reference.hlsl
Index: clang/test/AST/HLSL/this-reference.hlsl
===
--- clang/test/AST/HLSL/this-reference.hlsl
+++ clang/test/AST/HLSL/this-reference.hlsl
@@ -59,4 +59,4 @@
// CHECK-NEXT:`-ImplicitCastExpr 0x{{[0-9A-Fa-f]+}} 'int'
// CHECK-NEXT:`-MemberExpr 0x{{[0-9A-Fa-f]+}} 'int' lvalue .Second
0x{{[0-9A-Fa-f]+}}
// CHECK-NEXT:`-ImplicitCastExpr 0x{{[0-9A-Fa-f]+}} 'Pair' lvalue
-// CHECK-NEXT:`-CXXThisExpr 0x{{[0-9A-Fa-f]+}} 'PairInfo' lvalue
implicit this
\ No newline at end of file
+// CHECK-NEXT:`-CXXThisExpr 0x{{[0-9A-Fa-f]+}} 'PairInfo' lvalue
implicit this
Index: clang/test/AST/HLSL/this-reference.hlsl
===
--- clang/test/AST/HLSL/this-reference.hlsl
+++ clang/test/AST/HLSL/this-reference.hlsl
@@ -59,4 +59,4 @@
// CHECK-NEXT:`-ImplicitCastExpr 0x{{[0-9A-Fa-f]+}} 'int'
// CHECK-NEXT:`-MemberExpr 0x{{[0-9A-Fa-f]+}} 'int' lvalue .Second 0x{{[0-9A-Fa-f]+}}
// CHECK-NEXT:`-ImplicitCastExpr 0x{{[0-9A-Fa-f]+}} 'Pair' lvalue
-// CHECK-NEXT:`-CXXThisExpr 0x{{[0-9A-Fa-f]+}} 'PairInfo' lvalue implicit this
\ No newline at end of file
+// CHECK-NEXT:`-CXXThisExpr 0x{{[0-9A-Fa-f]+}} 'PairInfo' lvalue implicit this
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D135721: [HLSL] Added HLSL this as a reference
gracejennings updated this revision to Diff 466964.
gracejennings added a comment.
Includng formatting update
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135721/new/
https://reviews.llvm.org/D135721
Files:
clang/include/clang/AST/ExprCXX.h
clang/lib/AST/ExprClassification.cpp
clang/lib/CodeGen/CGExpr.cpp
clang/lib/Sema/HLSLExternalSemaSource.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/lib/Sema/SemaExprMember.cpp
clang/test/AST/HLSL/RWBuffer-AST.hlsl
clang/test/AST/HLSL/this-reference-template.hlsl
clang/test/AST/HLSL/this-reference.hlsl
Index: clang/test/AST/HLSL/this-reference.hlsl
===
--- /dev/null
+++ clang/test/AST/HLSL/this-reference.hlsl
@@ -0,0 +1,62 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -disable-llvm-passes -o - -hlsl-entry main %s | FileCheck %s
+
+class Pair {
+ int First;
+ int Second;
+
+ int getFirst() {
+ return this.First;
+ }
+
+ int getSecond() {
+return Second;
+ }
+};
+
+class PairInfo : Pair {
+ int Sum;
+
+ int getSum() {
+return this.First + Second;
+ }
+};
+
+[numthreads(1, 1, 1)]
+void main() {
+ Pair Vals = {1, 2};
+ Vals.First = Vals.getFirst();
+ Vals.Second = Vals.getSecond();
+
+ PairInfo ValsInfo;
+ ValsInfo.First = Vals.First;
+ ValsInfo.Second = Vals.Second;
+ ValsInfo.Sum = ValsInfo.getSum();
+
+}
+
+// CHECK: -CXXMethodDecl 0x{{[0-9A-Fa-f]+}} line:7:7 used getFirst 'int ()' implicit-inline
+// CHECK-NEXT:`-CompoundStmt 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:`-ReturnStmt 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:`-ImplicitCastExpr 0x{{[0-9A-Fa-f]+}} 'int'
+// CHECK-NEXT:`-MemberExpr 0x{{[0-9A-Fa-f]+}} 'int' lvalue .First 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:`-CXXThisExpr 0x{{[0-9A-Fa-f]+}} 'Pair' lvalue this
+// CHECK-NEXT:-CXXMethodDecl 0x{{[0-9A-Fa-f]+}} line:11:7 used getSecond 'int ()' implicit-inline
+// CHECK-NEXT:`-CompoundStmt 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:`-ReturnStmt 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:`-ImplicitCastExpr 0x{{[0-9A-Fa-f]+}} 'int'
+// CHECK-NEXT:`-MemberExpr 0x{{[0-9A-Fa-f]+}} 'int' lvalue .Second 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:`-CXXThisExpr 0x{{[0-9A-Fa-f]+}} 'Pair' lvalue implicit this
+
+
+// CHECK: CXXMethodDecl 0x{{[0-9A-Fa-f]+}} line:19:7 used getSum 'int ()' implicit-inline
+// CHECK-NEXT:`-CompoundStmt 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:`-ReturnStmt 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:`-BinaryOperator 0x{{[0-9A-Fa-f]+}} 'int' '+'
+// CHECK-NEXT:-ImplicitCastExpr 0x{{[0-9A-Fa-f]+}} 'int'
+// CHECK-NEXT:`-MemberExpr 0x{{[0-9A-Fa-f]+}} 'int' lvalue .First 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:`-ImplicitCastExpr 0x{{[0-9A-Fa-f]+}} 'Pair' lvalue
+// CHECK-NEXT:`-CXXThisExpr 0x{{[0-9A-Fa-f]+}} 'PairInfo' lvalue this
+// CHECK-NEXT:`-ImplicitCastExpr 0x{{[0-9A-Fa-f]+}} 'int'
+// CHECK-NEXT:`-MemberExpr 0x{{[0-9A-Fa-f]+}} 'int' lvalue .Second 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:`-ImplicitCastExpr 0x{{[0-9A-Fa-f]+}} 'Pair' lvalue
+// CHECK-NEXT:`-CXXThisExpr 0x{{[0-9A-Fa-f]+}} 'PairInfo' lvalue implicit this
Index: clang/test/AST/HLSL/this-reference-template.hlsl
===
--- /dev/null
+++ clang/test/AST/HLSL/this-reference-template.hlsl
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -disable-llvm-passes -o - -hlsl-entry main %s | FileCheck %s
+
+template
+struct Pair {
+ K First;
+ V Second;
+
+ K getFirst() {
+ return this.First;
+ }
+
+ V getSecond() {
+return Second;
+ }
+};
+
+[numthreads(1, 1, 1)]
+void main() {
+ Pair Vals = {1, 2.0};
+ Vals.First = Vals.getFirst();
+ Vals.Second = Vals.getSecond();
+}
+
+// CHECK: -CXXMethodDecl 0x{{[0-9A-Fa-f]+}} line:8:5 getFirst 'K ()' implicit-inline
+// CHECK-NEXT:-CompoundStmt 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:-ReturnStmt 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:-CXXDependentScopeMemberExpr 0x{{[0-9A-Fa-f]+}} '' lvalue .First
+// CHECK-NEXT:-CXXThisExpr 0x{{[0-9A-Fa-f]+}} 'Pair' lvalue this
+// CHECK-NEXT:-CXXMethodDecl 0x{{[0-9A-Fa-f]+}} line:12:5 getSecond 'V ()' implicit-inline
+// CHECK-NEXT:-CompoundStmt 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:-ReturnStmt 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:-MemberExpr 0x{{[0-9A-Fa-f]+}} 'V' lvalue .Second 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:-CXXThisExpr 0x{{[0-9A-Fa-f]+}} 'Pair' lvalue implicit this
+
+//CHECK: -CXXMethodDecl 0x{{[0-9A-Fa-f]+}} line:8:5 used getFirst 'int ()' implicit-inline
+// CHECK-NEXT:-CompoundStmt 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:-ReturnStmt 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:-ImplicitCastExpr 0x{{[0-9A-Fa-f]+}} 'int':'int'
+// CHECK-NEXT:-MemberExpr 0x{{[0-9A-Fa-f]+}} 'int':'int' lvalue .First 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT:-CXXThisExpr 0x{{[0-9A-Fa-f]+}} 'Pair' lvalue this
+// CHECK-NEXT:-CXXMethodDecl 0x{{[0-9A-Fa-f]+}} line:12:5 used getSecond 'float ()' implicit-inline
+// CHECK-NEXT:-Comp
[PATCH] D135721: [HLSL] Added HLSL this as a reference
gracejennings marked an inline comment as done. gracejennings added inline comments. Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:179 +AST, SourceLocation(), +Constructor->getThisType().getTypePtr()->getPointeeType(), true); +This->setValueKind(ExprValueKind::VK_LValue); python3kgae wrote: > Should this be a reference type? Could you expand on the question? I'm not sure I understand what you're asking. The two changes in this file were made to update the previous RWBuffer implementation Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135721/new/ https://reviews.llvm.org/D135721 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D135721: [HLSL] Added HLSL this as a reference
gracejennings added inline comments.
Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:179
+AST, SourceLocation(),
+Constructor->getThisType().getTypePtr()->getPointeeType(), true);
+This->setValueKind(ExprValueKind::VK_LValue);
python3kgae wrote:
> gracejennings wrote:
> > python3kgae wrote:
> > > Should this be a reference type?
> > Could you expand on the question? I'm not sure I understand what you're
> > asking. The two changes in this file were made to update the previous
> > RWBuffer implementation
> The current code will create CXXThisExpr with the pointeeType.
> I thought it should be a reference type of the pointeeType.
>
> Like in the test,
> CXXThisExpr 0x{{[0-9A-Fa-f]+}} <> 'RWBuffer *'
> implicit this
>
> The type is RWBuffer * before,
> I expected this patch will change it to
> RWBuffer &.
The change that makes it more reference like than c++ from:
`-MemberExpr 0x{{[0-9A-Fa-f]+}} 'int' lvalue ->First
0x{{[0-9A-Fa-f]+}}`
`-CXXThisExpr 0x{{[0-9A-Fa-f]+}} 'Pair *' this`
to hlsl with this change
`-MemberExpr 0x{{[0-9A-Fa-f]+}} 'int' lvalue .First
0x{{[0-9A-Fa-f]+}}`
`-CXXThisExpr 0x{{[0-9A-Fa-f]+}} 'Pair' lvalue this`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135721/new/
https://reviews.llvm.org/D135721
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D135721: [HLSL] Added HLSL this as a reference
gracejennings updated this revision to Diff 472791.
gracejennings marked an inline comment as done.
gracejennings added a comment.
- Add codegen test and fix member implicit this
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135721/new/
https://reviews.llvm.org/D135721
Files:
clang/lib/AST/ExprClassification.cpp
clang/lib/CodeGen/CGExpr.cpp
clang/lib/Sema/HLSLExternalSemaSource.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/lib/Sema/SemaExprMember.cpp
clang/test/AST/HLSL/RWBuffer-AST.hlsl
clang/test/AST/HLSL/this-reference-template.hlsl
clang/test/AST/HLSL/this-reference.hlsl
clang/test/CodeGenHLSL/this-assignment.hlsl
clang/test/CodeGenHLSL/this-reference.hlsl
Index: clang/test/CodeGenHLSL/this-reference.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/this-reference.hlsl
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -emit-llvm -disable-llvm-passes -o - -hlsl-entry main %s | FileCheck %s
+
+struct Pair {
+ int First;
+ float Second;
+
+ int getFirst() {
+ return this.First;
+ }
+
+ float getSecond() {
+return Second;
+ }
+};
+
+[numthreads(1, 1, 1)]
+void main() {
+ Pair Vals = {1, 2.0};
+ Vals.First = Vals.getFirst();
+ Vals.Second = Vals.getSecond();
+}
+
+// This tests reference like `this` in HLSL
+ // CHECK: %call = call noundef i32 @"?getFirst@Pair@@QAAHXZ"(ptr noundef nonnull align 4 dereferenceable(8) %Vals)
+ // CHECK-NEXT: %First = getelementptr inbounds %struct.Pair, ptr %Vals, i32 0, i32 0
+ // CHECK-NEXT: store i32 %call, ptr %First, align 4
+ // CHECK-NEXT: %call1 = call noundef float @"?getSecond@Pair@@QAAMXZ"(ptr noundef nonnull align 4 dereferenceable(8) %Vals)
+ // CHECK-NEXT: %Second = getelementptr inbounds %struct.Pair, ptr %Vals, i32 0, i32 1
Index: clang/test/CodeGenHLSL/this-assignment.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/this-assignment.hlsl
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -emit-llvm -disable-llvm-passes -o - -hlsl-entry main %s | FileCheck %s
+
+struct Pair {
+ int First;
+ int Second;
+
+ int getFirst() {
+Pair Another = {5, 10};
+this = Another;
+ return this.First;
+ }
+
+ int getSecond() {
+this = Pair();
+return Second;
+ }
+};
+
+[numthreads(1, 1, 1)]
+void main() {
+ Pair Vals = {1, 2.0};
+ Vals.First = Vals.getFirst();
+ Vals.Second = Vals.getSecond();
+}
+
+// This tests reference like implicit this in HLSL
+// CHECK: define linkonce_odr noundef i32 @"?getFirst@Pair@@QAAHXZ"(ptr noundef nonnull align 4 dereferenceable(8) %this) #3 align 2 {
+// CHECK-NEXT:entry:
+// CHECK-NEXT:%this.addr = alloca ptr, align 4
+// CHECK-NEXT:%Another = alloca %struct.Pair, align 4
+// CHECK-NEXT:store ptr %this, ptr %this.addr, align 4
+// CHECK-NEXT:%this1 = load ptr, ptr %this.addr, align 4
+// CHECK-NEXT:call void @llvm.memcpy.p0.p0.i32(ptr align 4 %Another, ptr align 4 @"__const.?getFirst@Pair@@QAAHXZ.Another", i32 8, i1 false)
+// CHECK-NEXT:call void @llvm.memcpy.p0.p0.i32(ptr align 4 %this1, ptr align 4 %Another, i32 8, i1 false)
+// CHECK-NEXT:%First = getelementptr inbounds %struct.Pair, ptr %this1, i32 0, i32 0
+
+// CHECK: define linkonce_odr noundef i32 @"?getSecond@Pair@@QAAHXZ"(ptr noundef nonnull align 4 dereferenceable(8) %this) #3 align 2 {
+// CHECK-NEXT:entry:
+// CHECK-NEXT:%this.addr = alloca ptr, align 4
+// CHECK-NEXT:%ref.tmp = alloca %struct.Pair, align 4
+// CHECK-NEXT:store ptr %this, ptr %this.addr, align 4
+// CHECK-NEXT:%this1 = load ptr, ptr %this.addr, align 4
+// CHECK-NEXT:call void @llvm.memset.p0.i32(ptr align 4 %ref.tmp, i8 0, i32 8, i1 false)
+// CHECK-NEXT:call void @llvm.memcpy.p0.p0.i32(ptr align 4 %this1, ptr align 4 %ref.tmp, i32 8, i1 false)
+// CHECK-NEXT:%Second = getelementptr inbounds %struct.Pair, ptr %this1, i32 0, i32 1
Index: clang/test/AST/HLSL/this-reference.hlsl
===
--- /dev/null
+++ clang/test/AST/HLSL/this-reference.hlsl
@@ -0,0 +1,62 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -disable-llvm-passes -o - -hlsl-entry main %s | FileCheck %s
+
+class Pair {
+ int First;
+ int Second;
+
+ int getFirst() {
+ return this.First;
+ }
+
+ int getSecond() {
+return Second;
+ }
+};
+
+class PairInfo : Pair {
+ int Sum;
+
+ int getSum() {
+return this.First + Second;
+ }
+};
+
+[numthreads(1, 1, 1)]
+void main() {
+ Pair Vals = {1, 2};
+ Vals.First = Vals.getFirst();
+ Vals.Second = Vals.getSecond();
+
+ PairInfo ValsInfo;
+ ValsInfo.First = Vals.First;
+ ValsInfo.Second = Vals.Second;
+ ValsInfo.Sum = ValsInfo.getSum();
+
+}
+
+// CHECK: -CXXMethodDecl 0x{{[0-9A-Fa-f]+}} line:7:7 used getFirst 'in
[PATCH] D135721: [HLSL] Added HLSL this as a reference
gracejennings marked an inline comment as done.
gracejennings added inline comments.
Comment at: clang/include/clang/AST/ExprCXX.h:1158-1161
+ static CXXThisExpr *Create(const ASTContext &C, SourceLocation Loc,
+ QualType Ty, bool IsImplicit) {
+return new (C) CXXThisExpr(Loc, Ty, IsImplicit);
+ }
aaron.ballman wrote:
> I don't think we need to add the `Create()` method -- but if there's a reason
> you really want to add it, I think we should protect the constructor so it's
> clear which interface callers should use. I think that sort of change is
> logically orthogonal to the rest of the patch and would be better as a
> separate (NFC) change.
Ok that makes a lot of sense. I'll go ahead and move it out of this change.
Comment at: clang/include/clang/AST/ExprCXX.h:1158-1161
+ static CXXThisExpr *Create(const ASTContext &C, SourceLocation Loc,
+ QualType Ty, bool IsImplicit) {
+return new (C) CXXThisExpr(Loc, Ty, IsImplicit);
+ }
gracejennings wrote:
> aaron.ballman wrote:
> > I don't think we need to add the `Create()` method -- but if there's a
> > reason you really want to add it, I think we should protect the constructor
> > so it's clear which interface callers should use. I think that sort of
> > change is logically orthogonal to the rest of the patch and would be better
> > as a separate (NFC) change.
> Ok that makes a lot of sense. I'll go ahead and move it out of this change.
Ok that makes sense, there was no compelling reason, so I'll go ahead and move
it out of this change.
Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:179
+AST, SourceLocation(),
+Constructor->getThisType().getTypePtr()->getPointeeType(), true);
+This->setValueKind(ExprValueKind::VK_LValue);
aaron.ballman wrote:
> python3kgae wrote:
> > gracejennings wrote:
> > > python3kgae wrote:
> > > > gracejennings wrote:
> > > > > python3kgae wrote:
> > > > > > gracejennings wrote:
> > > > > > > python3kgae wrote:
> > > > > > > > Should this be a reference type?
> > > > > > > Could you expand on the question? I'm not sure I understand what
> > > > > > > you're asking. The two changes in this file were made to update
> > > > > > > the previous RWBuffer implementation
> > > > > > The current code will create CXXThisExpr with the pointeeType.
> > > > > > I thought it should be a reference type of the pointeeType.
> > > > > >
> > > > > > Like in the test,
> > > > > > CXXThisExpr 0x{{[0-9A-Fa-f]+}} <>
> > > > > > 'RWBuffer *' implicit this
> > > > > >
> > > > > > The type is RWBuffer * before,
> > > > > > I expected this patch will change it to
> > > > > > RWBuffer &.
> > > > > The change that makes it more reference like than c++ from:
> > > > >
> > > > > `-MemberExpr 0x{{[0-9A-Fa-f]+}} 'int' lvalue ->First
> > > > > 0x{{[0-9A-Fa-f]+}}`
> > > > > `-CXXThisExpr 0x{{[0-9A-Fa-f]+}} 'Pair *' this`
> > > > >
> > > > > to hlsl with this change
> > > > >
> > > > > `-MemberExpr 0x{{[0-9A-Fa-f]+}} 'int' lvalue .First
> > > > > 0x{{[0-9A-Fa-f]+}}`
> > > > > `-CXXThisExpr 0x{{[0-9A-Fa-f]+}} 'Pair' lvalue this`
> > > > I guess we have to change clang codeGen for this anyway.
> > > >
> > > > Not sure which has less impact for codeGen side, lvalue like what is
> > > > in the current patch or make it a lvalue reference? My feeling is
> > > > lvalue reference might be eaiser.
> > > >
> > > > Did you test what needs to change for clang codeGen on top of the
> > > > current patch?
> > > >
> > > With just the lvalue change in the current patch there should be no
> > > additional changes needed in clang CodeGen on top of the current patch
> > Since we already get the codeGen working with this patch,
> > it would be nice to have a codeGen test.
> I think it's an interesting question to consider, but I have some concerns.
> Consider code like this:
> ```
> struct S {
> int Val = 0;
> void foo() {
> Val = 10;
> S Another;
> this = Another; // If this is a non-const reference, we can assign into
> it...
> print(); // ... so do we print 0 or 10?
> // When this function ends, is `this` destroyed because `Another` goes
> out of scope?
> }
> void print() {
> std::cout << Val;
> }
> };
> ```
> I think we need to prevent code like that from working. But it's not just
> direct assignments that are a concern. Consider:
> ```
> struct S;
>
> void func(S &Out, S &In) {
> Out = In;
> }
>
> struct S {
> int Val = 0;
> void foo() {
> Val = 10;
> S Another;
> func(this, Another); // Same problem here!
> print();
> }
> void print() {
> std::cout << Val;
> }
> };
> ```
> Another situation that I'm not certain of is whether HLSL supports
> tail-allocation where the code is doing something like `this + 1` to get to
> the start of the trailing objects.
For the first example we woul
[PATCH] D135721: [HLSL] Added HLSL this as a reference
gracejennings added inline comments.
Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:179
+AST, SourceLocation(),
+Constructor->getThisType().getTypePtr()->getPointeeType(), true);
+This->setValueKind(ExprValueKind::VK_LValue);
aaron.ballman wrote:
> beanz wrote:
> > aaron.ballman wrote:
> > > gracejennings wrote:
> > > > aaron.ballman wrote:
> > > > > python3kgae wrote:
> > > > > > gracejennings wrote:
> > > > > > > python3kgae wrote:
> > > > > > > > gracejennings wrote:
> > > > > > > > > python3kgae wrote:
> > > > > > > > > > gracejennings wrote:
> > > > > > > > > > > python3kgae wrote:
> > > > > > > > > > > > Should this be a reference type?
> > > > > > > > > > > Could you expand on the question? I'm not sure I
> > > > > > > > > > > understand what you're asking. The two changes in this
> > > > > > > > > > > file were made to update the previous RWBuffer
> > > > > > > > > > > implementation
> > > > > > > > > > The current code will create CXXThisExpr with the
> > > > > > > > > > pointeeType.
> > > > > > > > > > I thought it should be a reference type of the pointeeType.
> > > > > > > > > >
> > > > > > > > > > Like in the test,
> > > > > > > > > > CXXThisExpr 0x{{[0-9A-Fa-f]+}} <>
> > > > > > > > > > 'RWBuffer *' implicit this
> > > > > > > > > >
> > > > > > > > > > The type is RWBuffer * before,
> > > > > > > > > > I expected this patch will change it to
> > > > > > > > > > RWBuffer &.
> > > > > > > > > The change that makes it more reference like than c++ from:
> > > > > > > > >
> > > > > > > > > `-MemberExpr 0x{{[0-9A-Fa-f]+}} 'int' lvalue
> > > > > > > > > ->First 0x{{[0-9A-Fa-f]+}}`
> > > > > > > > > `-CXXThisExpr 0x{{[0-9A-Fa-f]+}} 'Pair *' this`
> > > > > > > > >
> > > > > > > > > to hlsl with this change
> > > > > > > > >
> > > > > > > > > `-MemberExpr 0x{{[0-9A-Fa-f]+}} 'int' lvalue
> > > > > > > > > .First 0x{{[0-9A-Fa-f]+}}`
> > > > > > > > > `-CXXThisExpr 0x{{[0-9A-Fa-f]+}} 'Pair' lvalue this`
> > > > > > > > I guess we have to change clang codeGen for this anyway.
> > > > > > > >
> > > > > > > > Not sure which has less impact for codeGen side, lvalue like
> > > > > > > > what is in the current patch or make it a lvalue reference? My
> > > > > > > > feeling is lvalue reference might be eaiser.
> > > > > > > >
> > > > > > > > Did you test what needs to change for clang codeGen on top of
> > > > > > > > the current patch?
> > > > > > > >
> > > > > > > With just the lvalue change in the current patch there should be
> > > > > > > no additional changes needed in clang CodeGen on top of the
> > > > > > > current patch
> > > > > > Since we already get the codeGen working with this patch,
> > > > > > it would be nice to have a codeGen test.
> > > > > I think it's an interesting question to consider, but I have some
> > > > > concerns. Consider code like this:
> > > > > ```
> > > > > struct S {
> > > > > int Val = 0;
> > > > > void foo() {
> > > > > Val = 10;
> > > > > S Another;
> > > > > this = Another; // If this is a non-const reference, we can
> > > > > assign into it...
> > > > > print(); // ... so do we print 0 or 10?
> > > > > // When this function ends, is `this` destroyed because `Another`
> > > > > goes out of scope?
> > > > > }
> > > > > void print() {
> > > > > std::cout << Val;
> > > > > }
> > > > > };
> > > > > ```
> > > > > I think we need to prevent code like that from working. But it's not
> > > > > just direct assignments that are a concern. Consider:
> > > > > ```
> > > > > struct S;
> > > > >
> > > > > void func(S &Out, S &In) {
> > > > > Out = In;
> > > > > }
> > > > >
> > > > > struct S {
> > > > > int Val = 0;
> > > > > void foo() {
> > > > > Val = 10;
> > > > > S Another;
> > > > > func(this, Another); // Same problem here!
> > > > > print();
> > > > > }
> > > > > void print() {
> > > > > std::cout << Val;
> > > > > }
> > > > > };
> > > > > ```
> > > > > Another situation that I'm not certain of is whether HLSL supports
> > > > > tail-allocation where the code is doing something like `this + 1` to
> > > > > get to the start of the trailing objects.
> > > > For the first example we would expect this to work for HLSL because
> > > > `this` is reference like (with modifications to make it supported by
> > > > HLSL). We would expect `Val` to be `0`, printing `0`, and `Another` to
> > > > be destroyed, but not `this`. I went ahead and added similar CodeGen
> > > > test coverage.
> > > >
> > > > For the second example, there is not reference support in HLSL.
> > > > Changing to a similar example with copy-in and copy-out to make it HLSL
> > > > supported would take care of that case, but copy-in/out is not
> > > > supported upstream yet.
> > > > For the first example we would expect this to work for HLSL because
> > > > this is reference like (with modifications to make it supported by
> > > > HLSL). We would expe
[PATCH] D135721: [HLSL] Added HLSL this as a reference
gracejennings updated this revision to Diff 473304.
gracejennings added a comment.
- Add assignment overload codegen test
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135721/new/
https://reviews.llvm.org/D135721
Files:
clang/lib/AST/ExprClassification.cpp
clang/lib/CodeGen/CGExpr.cpp
clang/lib/Sema/HLSLExternalSemaSource.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/lib/Sema/SemaExprMember.cpp
clang/test/AST/HLSL/RWBuffer-AST.hlsl
clang/test/AST/HLSL/this-reference-template.hlsl
clang/test/AST/HLSL/this-reference.hlsl
clang/test/CodeGenHLSL/this-assignment-overload.hlsl
clang/test/CodeGenHLSL/this-assignment.hlsl
clang/test/CodeGenHLSL/this-reference.hlsl
Index: clang/test/CodeGenHLSL/this-reference.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/this-reference.hlsl
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -emit-llvm -disable-llvm-passes -o - -hlsl-entry main %s | FileCheck %s
+
+struct Pair {
+ int First;
+ float Second;
+
+ int getFirst() {
+ return this.First;
+ }
+
+ float getSecond() {
+return Second;
+ }
+};
+
+[numthreads(1, 1, 1)]
+void main() {
+ Pair Vals = {1, 2.0};
+ Vals.First = Vals.getFirst();
+ Vals.Second = Vals.getSecond();
+}
+
+// This tests reference like `this` in HLSL
+ // CHECK: %call = call noundef i32 @"?getFirst@Pair@@QAAHXZ"(ptr noundef nonnull align 4 dereferenceable(8) %Vals)
+ // CHECK-NEXT: %First = getelementptr inbounds %struct.Pair, ptr %Vals, i32 0, i32 0
+ // CHECK-NEXT: store i32 %call, ptr %First, align 4
+ // CHECK-NEXT: %call1 = call noundef float @"?getSecond@Pair@@QAAMXZ"(ptr noundef nonnull align 4 dereferenceable(8) %Vals)
+ // CHECK-NEXT: %Second = getelementptr inbounds %struct.Pair, ptr %Vals, i32 0, i32 1
Index: clang/test/CodeGenHLSL/this-assignment.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/this-assignment.hlsl
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -emit-llvm -disable-llvm-passes -o - -hlsl-entry main %s | FileCheck %s
+
+struct Pair {
+ int First;
+ int Second;
+
+ int getFirst() {
+Pair Another = {5, 10};
+this = Another;
+ return this.First;
+ }
+
+ int getSecond() {
+this = Pair();
+return Second;
+ }
+};
+
+[numthreads(1, 1, 1)]
+void main() {
+ Pair Vals = {1, 2.0};
+ Vals.First = Vals.getFirst();
+ Vals.Second = Vals.getSecond();
+}
+
+// This tests reference like implicit this in HLSL
+// CHECK: define linkonce_odr noundef i32 @"?getFirst@Pair@@QAAHXZ"(ptr noundef nonnull align 4 dereferenceable(8) %this) #3 align 2 {
+// CHECK-NEXT:entry:
+// CHECK-NEXT:%this.addr = alloca ptr, align 4
+// CHECK-NEXT:%Another = alloca %struct.Pair, align 4
+// CHECK-NEXT:store ptr %this, ptr %this.addr, align 4
+// CHECK-NEXT:%this1 = load ptr, ptr %this.addr, align 4
+// CHECK-NEXT:call void @llvm.memcpy.p0.p0.i32(ptr align 4 %Another, ptr align 4 @"__const.?getFirst@Pair@@QAAHXZ.Another", i32 8, i1 false)
+// CHECK-NEXT:call void @llvm.memcpy.p0.p0.i32(ptr align 4 %this1, ptr align 4 %Another, i32 8, i1 false)
+// CHECK-NEXT:%First = getelementptr inbounds %struct.Pair, ptr %this1, i32 0, i32 0
+
+// CHECK: define linkonce_odr noundef i32 @"?getSecond@Pair@@QAAHXZ"(ptr noundef nonnull align 4 dereferenceable(8) %this) #3 align 2 {
+// CHECK-NEXT:entry:
+// CHECK-NEXT:%this.addr = alloca ptr, align 4
+// CHECK-NEXT:%ref.tmp = alloca %struct.Pair, align 4
+// CHECK-NEXT:store ptr %this, ptr %this.addr, align 4
+// CHECK-NEXT:%this1 = load ptr, ptr %this.addr, align 4
+// CHECK-NEXT:call void @llvm.memset.p0.i32(ptr align 4 %ref.tmp, i8 0, i32 8, i1 false)
+// CHECK-NEXT:call void @llvm.memcpy.p0.p0.i32(ptr align 4 %this1, ptr align 4 %ref.tmp, i32 8, i1 false)
+// CHECK-NEXT:%Second = getelementptr inbounds %struct.Pair, ptr %this1, i32 0, i32 1
Index: clang/test/CodeGenHLSL/this-assignment-overload.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/this-assignment-overload.hlsl
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -emit-llvm -disable-llvm-passes -o - -std=hlsl202x %s | FileCheck %s
+
+struct Pair {
+ int First;
+ int Second;
+ int getFirst() {
+Pair Another = {5, 10};
+this = Another;
+ return this.First;
+ }
+ int getSecond() {
+this = Pair();
+return Second;
+ }
+ void operator=(Pair P) {
+First = P.First;
+Second = 2;
+ }
+};
+[numthreads(1, 1, 1)]
+void main() {
+ Pair Vals = {1, 2};
+ Vals.First = Vals.getFirst();
+ Vals.Second = Vals.getSecond();
+}
+
+// This test makes a probably safe assumption that HLSL 202x includes operator overloading for assignment operators.
+// CHECK: define linkonce_odr
