Skip to content

Commit 164c6af

Browse files
joaosaffran-zzjoaosaffran-zz
committed
addressing comments
1 parent a676944 commit 164c6af

File tree

5 files changed

+72
-129
lines changed

5 files changed

+72
-129
lines changed

‎clang/lib/CodeGen/CGBuiltin.cpp

Lines changed: 19 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -132,23 +132,6 @@ static Value *handleHlslSplitdouble(const CallExpr *E, CodeGenFunction *CGF) {
132132

133133
} else {
134134
// For Non DXIL targets we generate the instructions.
135-
// TODO: This code accounts for known limitations in
136-
// SPIR-V and splitdouble. Such should be handled,
137-
// in a later compilation stage. After
138-
// https://github.com/llvm/llvm-project/issues/113597 is fixed, this shall
139-
// be refactored.
140-
141-
// casts `<2 x double>` to `<4 x i32>`, then shuffles into high and low
142-
// `<2 x i32>` vectors.
143-
auto EmitDouble2Cast =
144-
[](CodeGenFunction &CGF,
145-
Value *DoubleVec2) -> std::pair<Value *, Value *> {
146-
Value *BC = CGF.Builder.CreateBitCast(
147-
DoubleVec2, FixedVectorType::get(CGF.Int32Ty, 4));
148-
Value *LB = CGF.Builder.CreateShuffleVector(BC, {0, 2});
149-
Value *HB = CGF.Builder.CreateShuffleVector(BC, {1, 3});
150-
return std::make_pair(LB, HB);
151-
};
152135

153136
if (!Op0->getType()->isVectorTy()) {
154137
FixedVectorType *DestTy = FixedVectorType::get(CGF->Int32Ty, 2);
@@ -157,58 +140,25 @@ static Value *handleHlslSplitdouble(const CallExpr *E, CodeGenFunction *CGF) {
157140
LowBits = CGF->Builder.CreateExtractElement(Bitcast, (uint64_t)0);
158141
HighBits = CGF->Builder.CreateExtractElement(Bitcast, 1);
159142
} else {
160-
161-
const auto *TargTy = E->getArg(0)->getType()->getAs<clang::VectorType>();
162-
163-
int NumElements = TargTy->getNumElements();
164-
165-
FixedVectorType *UintVec2 = FixedVectorType::get(CGF->Int32Ty, 2);
166-
167-
switch (NumElements) {
168-
case 1: {
169-
auto *Bitcast = CGF->Builder.CreateBitCast(Op0, UintVec2);
170-
171-
LowBits = CGF->Builder.CreateExtractElement(Bitcast, (uint64_t)0);
172-
HighBits = CGF->Builder.CreateExtractElement(Bitcast, 1);
173-
break;
174-
}
175-
case 2: {
176-
auto [LB, HB] = EmitDouble2Cast(*CGF, Op0);
177-
LowBits = LB;
178-
HighBits = HB;
179-
break;
180-
}
181-
182-
case 3: {
183-
auto *Shuff = CGF->Builder.CreateShuffleVector(Op0, {0, 1});
184-
auto [LB, HB] = EmitDouble2Cast(*CGF, Shuff);
185-
186-
auto *EV = CGF->Builder.CreateExtractElement(Op0, 2);
187-
auto *ScalarBitcast = CGF->Builder.CreateBitCast(EV, UintVec2);
188-
189-
LowBits =
190-
CGF->Builder.CreateShuffleVector(LB, ScalarBitcast, {0, 1, 2});
191-
HighBits =
192-
CGF->Builder.CreateShuffleVector(HB, ScalarBitcast, {0, 1, 3});
193-
break;
194-
}
195-
case 4: {
196-
197-
auto *Shuff1 = CGF->Builder.CreateShuffleVector(Op0, {0, 1});
198-
auto [LB1, HB1] = EmitDouble2Cast(*CGF, Shuff1);
199-
200-
auto *Shuff2 = CGF->Builder.CreateShuffleVector(Op0, {2, 3});
201-
auto [LB2, HB2] = EmitDouble2Cast(*CGF, Shuff2);
202-
203-
LowBits = CGF->Builder.CreateShuffleVector(LB1, LB2, {0, 1, 2, 3});
204-
HighBits = CGF->Builder.CreateShuffleVector(HB1, HB2, {0, 1, 3, 3});
205-
break;
206-
}
207-
default: {
208-
CGF->CGM.Error(E->getExprLoc(),
209-
"splitdouble doesn't support vectors larger than 4.");
210-
return nullptr;
211-
}
143+
int NumElements = 1;
144+
if (const auto *VecTy =
145+
E->getArg(0)->getType()->getAs<clang::VectorType>())
146+
NumElements = VecTy->getNumElements();
147+
148+
FixedVectorType *Uint32VecTy =
149+
FixedVectorType::get(CGF->Int32Ty, NumElements * 2);
150+
Value *Uint32Vec = CGF->Builder.CreateBitCast(Op0, Uint32VecTy);
151+
if (NumElements == 1) {
152+
LowBits = CGF->Builder.CreateExtractElement(Uint32Vec, (uint64_t)0);
153+
HighBits = CGF->Builder.CreateExtractElement(Uint32Vec, 1);
154+
} else {
155+
SmallVector<int> EvenMask, OddMask;
156+
for (int I = 0, E = NumElements; I != E; ++I) {
157+
EvenMask.push_back(I * 2);
158+
OddMask.push_back(I * 2 + 1);
159+
}
160+
LowBits = CGF->Builder.CreateShuffleVector(Uint32Vec, EvenMask);
161+
HighBits = CGF->Builder.CreateShuffleVector(Uint32Vec, OddMask);
212162
}
213163
}
214164
}

‎clang/lib/Sema/SemaHLSL.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1748,9 +1748,11 @@ static bool CheckFloatOrHalfRepresentations(Sema *S, CallExpr *TheCall) {
17481748
static bool CheckModifiableLValue(Sema *S, CallExpr *TheCall,
17491749
unsigned ArgIndex) {
17501750
auto *Arg = TheCall->getArg(ArgIndex);
1751-
if (Arg->isModifiableLvalue(S->getASTContext()) == Expr::MLV_Valid)
1751+
SourceLocation OrigLoc = Arg->getExprLoc();
1752+
if (Arg->IgnoreCasts()->isModifiableLvalue(S->Context, &OrigLoc) ==
1753+
Expr::MLV_Valid)
17521754
return false;
1753-
S->Diag(Arg->getBeginLoc(), diag::error_hlsl_inout_lvalue) << Arg << 0;
1755+
S->Diag(OrigLoc, diag::error_hlsl_inout_lvalue) << Arg << 0;
17541756
return true;
17551757
}
17561758

@@ -2102,10 +2104,10 @@ bool SemaHLSL::CheckBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall) {
21022104
CheckScalarOrVector(&SemaRef, TheCall, SemaRef.Context.UnsignedIntTy,
21032105
2))
21042106
return true;
2107+
21052108
if (CheckModifiableLValue(&SemaRef, TheCall, 1) ||
21062109
CheckModifiableLValue(&SemaRef, TheCall, 2))
21072110
return true;
2108-
21092111
break;
21102112
}
21112113
case Builtin::BI__builtin_elementwise_acos:

‎clang/test/CodeGenHLSL/builtins/splitdouble.hlsl

Lines changed: 10 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// RUN: %clang_cc1 -finclude-default-header -x hlsl -triple dxil-pc-shadermodel6.3-library %s -fnative-half-type -emit-llvm -O1 -o - | FileCheck %s
2-
// RUN: %clang_cc1 -finclude-default-header -x hlsl -triple spirv-vulkan-library %s -fnative-half-type -emit-llvm -O0 -o - | FileCheck %s --check-prefix=SPIRV
2+
// RUN: %clang_cc1 -finclude-default-header -x hlsl -triple spirv-vulkan-library %s -fnative-half-type -emit-llvm -O1 -o - | FileCheck %s --check-prefix=SPIRV
33

44

55

@@ -9,8 +9,7 @@
99
// CHECK-NEXT: extractvalue { i32, i32 } [[VALRET]], 1
1010
// SPIRV: define spir_func {{.*}} i32 {{.*}}test_scalar{{.*}}(double {{.*}} [[VALD:%.*]])
1111
// SPIRV-NOT: @llvm.dx.splitdouble.i32
12-
// SPIRV: [[REG:%.*]] = load double, ptr [[VALD]].addr, align 8
13-
// SPIRV-NEXT: [[CAST:%.*]] = bitcast double [[REG]] to <2 x i32>
12+
// SPIRV: [[CAST:%.*]] = bitcast double [[VALD]] to <2 x i32>
1413
// SPIRV-NEXT: extractelement <2 x i32> [[CAST]], i64 0
1514
// SPIRV-NEXT: extractelement <2 x i32> [[CAST]], i64 1
1615
uint test_scalar(double D) {
@@ -26,9 +25,7 @@ uint test_scalar(double D) {
2625
// CHECK-NEXT: extractvalue { i32, i32 } [[VALRET]], 1
2726
// SPIRV: define spir_func {{.*}} <1 x i32> {{.*}}test_double1{{.*}}(<1 x double> {{.*}} [[VALD:%.*]])
2827
// SPIRV-NOT: @llvm.dx.splitdouble.i32
29-
// SPIRV: [[REG:%.*]] = load <1 x double>, ptr [[VALD]].addr, align 8
30-
// SPIRV-NEXT: [[TRUNC:%.*]] = extractelement <1 x double> %1, i64 0
31-
// SPIRV-NEXT: [[CAST:%.*]] = bitcast double [[TRUNC]] to <2 x i32>
28+
// SPIRV: [[CAST:%.*]] = bitcast <1 x double> [[VALD]] to <2 x i32>
3229
// SPIRV-NEXT: extractelement <2 x i32> [[CAST]], i64 0
3330
// SPIRV-NEXT: extractelement <2 x i32> [[CAST]], i64 1
3431
uint1 test_double1(double1 D) {
@@ -43,8 +40,7 @@ uint1 test_double1(double1 D) {
4340
// CHECK-NEXT: extractvalue { <2 x i32>, <2 x i32> } [[VALRET]], 1
4441
// SPIRV: define spir_func {{.*}} <2 x i32> {{.*}}test_vector2{{.*}}(<2 x double> {{.*}} [[VALD:%.*]])
4542
// SPIRV-NOT: @llvm.dx.splitdouble.i32
46-
// SPIRV: [[REG:%.*]] = load <2 x double>, ptr [[VALD]].addr, align 16
47-
// SPIRV-NEXT: [[CAST1:%.*]] = bitcast <2 x double> [[REG]] to <4 x i32>
43+
// SPIRV: [[CAST1:%.*]] = bitcast <2 x double> [[VALD]] to <4 x i32>
4844
// SPIRV-NEXT: [[SHUF1:%.*]] = shufflevector <4 x i32> [[CAST1]], <4 x i32> poison, <2 x i32> <i32 0, i32 2>
4945
// SPIRV-NEXT: [[SHUF2:%.*]] = shufflevector <4 x i32> [[CAST1]], <4 x i32> poison, <2 x i32> <i32 1, i32 3>
5046
uint2 test_vector2(double2 D) {
@@ -59,15 +55,9 @@ uint2 test_vector2(double2 D) {
5955
// CHECK-NEXT: extractvalue { <3 x i32>, <3 x i32> } [[VALRET]], 1
6056
// SPIRV: define spir_func {{.*}} <3 x i32> {{.*}}test_vector3{{.*}}(<3 x double> {{.*}} [[VALD:%.*]])
6157
// SPIRV-NOT: @llvm.dx.splitdouble.i32
62-
// SPIRV: [[REG:%.*]] = load <3 x double>, ptr [[VALD]].addr, align 32
63-
// SPIRV-NEXT: [[VALRET1:%.*]] = shufflevector <3 x double> [[REG]], <3 x double> poison, <2 x i32> <i32 0, i32 1>
64-
// SPIRV-NEXT: [[CAST1:%.*]] = bitcast <2 x double> [[VALRET1]] to <4 x i32>
65-
// SPIRV-NEXT: [[SHUF1:%.*]] = shufflevector <4 x i32> [[CAST1]], <4 x i32> poison, <2 x i32> <i32 0, i32 2>
66-
// SPIRV-NEXT: [[SHUF2:%.*]] = shufflevector <4 x i32> [[CAST1]], <4 x i32> poison, <2 x i32> <i32 1, i32 3>
67-
// SPIRV-NEXT: [[EXTRACT:%.*]] = extractelement <3 x double> [[REG]], i64 2
68-
// SPIRV-NEXT: [[CAST:%.*]] = bitcast double [[EXTRACT]] to <2 x i32>
69-
// SPIRV-NEXT: %[[#]] = shufflevector <2 x i32> [[SHUF1]], <2 x i32> [[CAST]], <3 x i32> <i32 0, i32 1, i32 2>
70-
// SPIRV-NEXT: %[[#]] = shufflevector <2 x i32> [[SHUF2]], <2 x i32> [[CAST]], <3 x i32> <i32 0, i32 1, i32 3>
58+
// SPIRV: [[CAST1:%.*]] = bitcast <3 x double> [[VALD]] to <6 x i32>
59+
// SPIRV-NEXT: [[SHUF1:%.*]] = shufflevector <6 x i32> [[CAST1]], <6 x i32> poison, <3 x i32> <i32 0, i32 2, i32 4>
60+
// SPIRV-NEXT: [[SHUF2:%.*]] = shufflevector <6 x i32> [[CAST1]], <6 x i32> poison, <3 x i32> <i32 1, i32 3, i32 5>
7161
uint3 test_vector3(double3 D) {
7262
uint3 A, B;
7363
asuint(D, A, B);
@@ -80,18 +70,9 @@ uint3 test_vector3(double3 D) {
8070
// CHECK-NEXT: extractvalue { <4 x i32>, <4 x i32> } [[VALRET]], 1
8171
// SPIRV: define spir_func {{.*}} <4 x i32> {{.*}}test_vector4{{.*}}(<4 x double> {{.*}} [[VALD:%.*]])
8272
// SPIRV-NOT: @llvm.dx.splitdouble.i32
83-
// SPIRV: [[REG:%.*]] = load <4 x double>, ptr [[VALD]].addr, align 32
84-
// SPIRV-NEXT: [[VALRET1:%.*]] = shufflevector <4 x double> [[REG]], <4 x double> poison, <2 x i32> <i32 0, i32 1>
85-
// SPIRV-NEXT: [[CAST1:%.*]] = bitcast <2 x double> [[VALRET1]] to <4 x i32>
86-
// SPIRV-NEXT: [[SHUF1:%.*]] = shufflevector <4 x i32> [[CAST1]], <4 x i32> poison, <2 x i32> <i32 0, i32 2>
87-
// SPIRV-NEXT: [[SHUF2:%.*]] = shufflevector <4 x i32> [[CAST1]], <4 x i32> poison, <2 x i32> <i32 1, i32 3>
88-
// SPIRV-NEXT: [[VALRET2:%.*]] = shufflevector <4 x double> [[REG]], <4 x double> poison, <2 x i32> <i32 2, i32 3>
89-
// SPIRV-NEXT: [[CAST2:%.*]] = bitcast <2 x double> [[VALRET2]] to <4 x i32>
90-
// SPIRV-NEXT: [[SHUF3:%.*]] = shufflevector <4 x i32> [[CAST2]], <4 x i32> poison, <2 x i32> <i32 0, i32 2>
91-
// SPIRV-NEXT: [[SHUF4:%.*]] = shufflevector <4 x i32> [[CAST2]], <4 x i32> poison, <2 x i32> <i32 1, i32 3>
92-
// SPIRV-NEXT: shufflevector <2 x i32> [[SHUF1]], <2 x i32> [[SHUF3]], <4 x i32> <i32 0, i32 1, i32 2, i32 3>
93-
// SPIRV-NEXT: shufflevector <2 x i32> [[SHUF2]], <2 x i32> [[SHUF4]], <4 x i32> <i32 0, i32 1, i32 3, i32 3>
94-
73+
// SPIRV: [[CAST1:%.*]] = bitcast <4 x double> [[VALD]] to <8 x i32>
74+
// SPIRV-NEXT: [[SHUF1:%.*]] = shufflevector <8 x i32> [[CAST1]], <8 x i32> poison, <4 x i32> <i32 0, i32 2, i32 4, i32 6>
75+
// SPIRV-NEXT: [[SHUF2:%.*]] = shufflevector <8 x i32> [[CAST1]], <8 x i32> poison, <4 x i32> <i32 1, i32 3, i32 5, i32 7>
9576
uint4 test_vector4(double4 D) {
9677
uint4 A, B;
9778
asuint(D, A, B);

‎clang/test/SemaHLSL/BuiltIns/splitdouble-errors.hlsl

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,36 @@ void test_const_second_arg(double D) {
4141
const uint A = 1;
4242
uint B;
4343
__builtin_hlsl_elementwise_splitdouble(D, A, B);
44-
// expected-error@-1 {{cannot bind non-lvalue argument}}
44+
// expected-error@-1 {{cannot bind non-lvalue argument A to out paramemter}}
4545
}
4646

4747
void test_const_third_arg(double D) {
4848
uint A;
49-
const uint B = 2;
49+
const uint B = 1;
5050
__builtin_hlsl_elementwise_splitdouble(D, A, B);
51-
// expected-error@-1 {{cannot bind non-lvalue argument}}
51+
// expected-error@-1 {{cannot bind non-lvalue argument B to out paramemter}}
52+
}
53+
54+
void test_number_second_arg(double D) {
55+
uint B;
56+
__builtin_hlsl_elementwise_splitdouble(D, (uint)1, B);
57+
// expected-error@-1 {{cannot bind non-lvalue argument (uint)1 to out paramemter}}
58+
}
59+
60+
void test_number_third_arg(double D) {
61+
uint B;
62+
__builtin_hlsl_elementwise_splitdouble(D, B, (uint)1);
63+
// expected-error@-1 {{cannot bind non-lvalue argument (uint)1 to out paramemter}}
64+
}
65+
66+
void test_expr_second_arg(double D) {
67+
uint B;
68+
__builtin_hlsl_elementwise_splitdouble(D, B+1, B);
69+
// expected-error@-1 {{cannot bind non-lvalue argument B + 1 to out paramemter}}
70+
}
71+
72+
void test_expr_third_arg(double D) {
73+
uint B;
74+
__builtin_hlsl_elementwise_splitdouble(D, B, B+1);
75+
// expected-error@-1 {{cannot bind non-lvalue argument B + 1 to out paramemter}}
5276
}

‎llvm/test/CodeGen/SPIRV/hlsl-intrinsics/splitdouble.ll

Lines changed: 11 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,10 @@
44
; Make sure lowering is correctly generating spirv code.
55

66
; CHECK-DAG: %[[#double:]] = OpTypeFloat 64
7+
; CHECK-DAG: %[[#vec_2_double:]] = OpTypeVector %[[#double]] 2
78
; CHECK-DAG: %[[#int_32:]] = OpTypeInt 32 0
8-
; CHECK-DAG: %[[#scalar_function:]] = OpTypeFunction %[[#int_32]] %[[#double]]
99
; CHECK-DAG: %[[#vec_2_int_32:]] = OpTypeVector %[[#int_32]] 2
1010
; CHECK-DAG: %[[#vec_4_int_32:]] = OpTypeVector %[[#int_32]] 4
11-
; CHECK-DAG: %[[#vec_3_int_32:]] = OpTypeVector %[[#int_32]] 3
12-
; CHECK-DAG: %[[#vec_3_double:]] = OpTypeVector %[[#double]] 3
13-
; CHECK-DAG: %[[#vector_function:]] = OpTypeFunction %[[#vec_3_int_32]] %[[#vec_3_double]]
14-
; CHECK-DAG: %[[#vec_2_double:]] = OpTypeVector %[[#double]] 2
1511

1612

1713
define spir_func noundef i32 @test_scalar(double noundef %D) local_unnamed_addr {
@@ -29,26 +25,16 @@ entry:
2925
}
3026

3127

32-
define spir_func noundef <3 x i32> @test_vector(<3 x double> noundef %D) local_unnamed_addr {
28+
define spir_func noundef <2 x i32> @test_vector(<2 x double> noundef %D) local_unnamed_addr {
3329
entry:
3430
; CHECK-LABEL: ; -- Begin function test_vector
35-
; CHECK: %[[#param:]] = OpFunctionParameter %[[#vec_3_double]]
36-
; %[[#SHUFF1:]] = OpVectorShuffle %[[#vec_2_double]] %[[#param]] %[[#]] 0 1
37-
; %[[#CAST1:]] = OpBitcast %[[#vec_4_int_32]] %[[#SHUFF1]]
38-
; %[[#SHUFF2:]] = OpVectorShuffle %[[#vec_2_int_32]] %[[#CAST1]] %[[#]] 0 2
39-
; %[[#SHUFF3:]] = OpVectorShuffle %[[#vec_2_int_32]] %[[#CAST1]] %[[#]] 1 3
40-
; %[[#EXTRACT:]] = OpCompositeExtract %[[#double]] %[[#param]] 2
41-
; %[[#CAST2:]] = OpBitcast %[[#vec_2_int_32]] %[[#EXTRACT]]
42-
; %[[#]] = OpVectorShuffle %7 %[[#SHUFF2]] %[[#CAST2]] 0 1 2
43-
; %[[#]] = OpVectorShuffle %7 %[[#SHUFF3]] %[[#CAST2]] 0 1 3
44-
%0 = shufflevector <3 x double> %D, <3 x double> poison, <2 x i32> <i32 0, i32 1>
45-
%1 = bitcast <2 x double> %0 to <4 x i32>
46-
%2 = shufflevector <4 x i32> %1, <4 x i32> poison, <2 x i32> <i32 0, i32 2>
47-
%3 = shufflevector <4 x i32> %1, <4 x i32> poison, <2 x i32> <i32 1, i32 3>
48-
%4 = extractelement <3 x double> %D, i64 2
49-
%5 = bitcast double %4 to <2 x i32>
50-
%6 = shufflevector <2 x i32> %2, <2 x i32> %5, <3 x i32> <i32 0, i32 1, i32 2>
51-
%7 = shufflevector <2 x i32> %3, <2 x i32> %5, <3 x i32> <i32 0, i32 1, i32 3>
52-
%add = add <3 x i32> %6, %7
53-
ret <3 x i32> %add
31+
; CHECK: %[[#param:]] = OpFunctionParameter %[[#vec_2_double]]
32+
; CHECK: %[[#CAST1:]] = OpBitcast %[[#vec_4_int_32]] %[[#param]]
33+
; CHECK: %[[#SHUFF2:]] = OpVectorShuffle %[[#vec_2_int_32]] %[[#CAST1]] %[[#]] 0 2
34+
; CHECK: %[[#SHUFF3:]] = OpVectorShuffle %[[#vec_2_int_32]] %[[#CAST1]] %[[#]] 1 3
35+
%0 = bitcast <2 x double> %D to <4 x i32>
36+
%1 = shufflevector <4 x i32> %0, <4 x i32> poison, <2 x i32> <i32 0, i32 2>
37+
%2 = shufflevector <4 x i32> %0, <4 x i32> poison, <2 x i32> <i32 1, i32 3>
38+
%add = add <2 x i32> %1, %2
39+
ret <2 x i32> %add
5440
}

0 commit comments

Comments
 (0)