Skip to content

Commit

Permalink
[HLSL] Fix move assignment of this (llvm#108445)
Browse files Browse the repository at this point in the history
Under HLSL 202x+ move assignment can occur and when targeting `this`
move assignment was generating some really odd errors. This corrects the
errors by properly generating the `this` object reference for HLSL and
always treating it as a reference.

This mirrors the implementation added eariler for copy assignment, and
extends the test case to cover both move and copy assignment under HLSL
202x+.
  • Loading branch information
llvm-beanz authored Sep 13, 2024
1 parent 77bab2a commit 03618ce
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 7 deletions.
10 changes: 6 additions & 4 deletions clang/lib/Sema/SemaDeclCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15331,6 +15331,7 @@ void Sema::DefineImplicitMoveAssignment(SourceLocation CurrentLocation,
std::optional<DerefBuilder> DerefThis;
std::optional<RefBuilder> ExplicitObject;
QualType ObjectType;
bool IsArrow = false;
if (MoveAssignOperator->isExplicitObjectMemberFunction()) {
ObjectType = MoveAssignOperator->getParamDecl(0)->getType();
if (ObjectType->isReferenceType())
Expand All @@ -15340,6 +15341,7 @@ void Sema::DefineImplicitMoveAssignment(SourceLocation CurrentLocation,
ObjectType = getCurrentThisType();
This.emplace();
DerefThis.emplace(*This);
IsArrow = !getLangOpts().HLSL;
}
ExprBuilder &ObjectParameter =
ExplicitObject ? *ExplicitObject : static_cast<ExprBuilder &>(*This);
Expand Down Expand Up @@ -15441,8 +15443,7 @@ void Sema::DefineImplicitMoveAssignment(SourceLocation CurrentLocation,
MemberLookup.resolveKind();
MemberBuilder From(MoveOther, OtherRefType,
/*IsArrow=*/false, MemberLookup);
MemberBuilder To(ObjectParameter, ObjectType, /*IsArrow=*/!ExplicitObject,
MemberLookup);
MemberBuilder To(ObjectParameter, ObjectType, IsArrow, MemberLookup);

assert(!From.build(*this, Loc)->isLValue() && // could be xvalue or prvalue
"Member reference with rvalue base must be rvalue except for reference "
Expand All @@ -15465,8 +15466,9 @@ void Sema::DefineImplicitMoveAssignment(SourceLocation CurrentLocation,
if (!Invalid) {
// Add a "return *this;"
Expr *ThisExpr =
(ExplicitObject ? static_cast<ExprBuilder &>(*ExplicitObject)
: static_cast<ExprBuilder &>(*DerefThis))
(ExplicitObject ? static_cast<ExprBuilder &>(*ExplicitObject)
: LangOpts.HLSL ? static_cast<ExprBuilder &>(*This)
: static_cast<ExprBuilder &>(*DerefThis))
.build(*this, Loc);

StmtResult Return = BuildReturnStmt(Loc, ThisExpr);
Expand Down
30 changes: 27 additions & 3 deletions clang/test/CodeGenHLSL/this-assignment.hlsl
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -emit-llvm -disable-llvm-passes -o - -hlsl-entry main %s | FileCheck %s
// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -emit-llvm -disable-llvm-passes -o - -hlsl-entry main %s | FileCheck %s
// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -std=hlsl202x -emit-llvm -disable-llvm-passes -o - -hlsl-entry main %s | FileCheck %s

struct Pair {
int First;
Expand All @@ -10,21 +11,30 @@ struct Pair {
return this.First;
}

// In HLSL 202x, this is a move assignment rather than a copy.
int getSecond() {
this = Pair();
return Second;
}

// In HLSL 202x, this is a copy assignment.
Pair DoSilly(Pair Obj) {
this = Obj;
First += 2;
return Obj;
}
};

[numthreads(1, 1, 1)]
void main() {
Pair Vals = {1, 2.0};
Vals.First = Vals.getFirst();
Vals.Second = Vals.getSecond();
(void) Vals.DoSilly(Vals);
}

// 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) #0 align 2 {
// CHECK-LABEL: define {{.*}}getFirst
// CHECK-NEXT:entry:
// CHECK-NEXT:%this.addr = alloca ptr, align 4
// CHECK-NEXT:%Another = alloca %struct.Pair, align 4
Expand All @@ -34,7 +44,7 @@ void main() {
// 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 nuw %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) #0 align 2 {
// CHECK-LABEL: define {{.*}}getSecond
// CHECK-NEXT:entry:
// CHECK-NEXT:%this.addr = alloca ptr, align 4
// CHECK-NEXT:%ref.tmp = alloca %struct.Pair, align 4
Expand All @@ -43,3 +53,17 @@ void main() {
// 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 nuw %struct.Pair, ptr %this1, i32 0, i32 1

// CHECK-LABEL: define {{.*}}DoSilly
// CHECK-NEXT:entry:
// CHECK-NEXT: [[ResPtr:%.*]] = alloca ptr
// CHECK-NEXT: [[ThisPtrAddr:%.*]] = alloca ptr
// CHECK-NEXT: store ptr [[AggRes:%.*]], ptr [[ResPtr]]
// CHECK-NEXT: store ptr {{.*}}, ptr [[ThisPtrAddr]]
// CHECK-NEXT: [[ThisPtr:%.*]] = load ptr, ptr [[ThisPtrAddr]]
// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[ThisPtr]], ptr align 4 [[Obj:%.*]], i32 8, i1 false)
// CHECK-NEXT: [[FirstAddr:%.*]] = getelementptr inbounds nuw %struct.Pair, ptr [[ThisPtr]], i32 0, i32 0
// CHECK-NEXT: [[First:%.*]] = load i32, ptr [[FirstAddr]]
// CHECK-NEXT: [[FirstPlusTwo:%.*]] = add nsw i32 [[First]], 2
// CHECK-NEXT: store i32 [[FirstPlusTwo]], ptr [[FirstAddr]]
// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[AggRes]], ptr align 4 [[Obj]], i32 8, i1 false)

0 comments on commit 03618ce

Please sign in to comment.