Skip to content

Commit 9c23ce9

Browse files
spallspall
committed
respond to pr feedback
1 parent 2ae441e commit 9c23ce9

File tree

2 files changed

+89
-33
lines changed

2 files changed

+89
-33
lines changed

‎llvm/lib/Target/DirectX/DXILIntrinsicExpansion.cpp‎

Lines changed: 38 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -541,29 +541,35 @@ static Value *expandRadiansIntrinsic(CallInst *Orig) {
541541
return Builder.CreateFMul(X, PiOver180);
542542
}
543543

544-
static void expandTypedBufferLoadIntrinsic(CallInst *Orig) {
544+
static Value *expandTypedBufferLoadIntrinsic(CallInst *Orig) {
545545
IRBuilder<> Builder(Orig);
546546

547-
unsigned ExtractNum =
548-
Orig->getType()->getStructElementType(0)->isVectorTy() ? 4 : 2;
547+
Type *BufferTy = Orig->getType()->getStructElementType(0);
548+
549+
unsigned ExtractNum = 2;
550+
if (auto *VT = dyn_cast<FixedVectorType>(BufferTy)) {
551+
assert(VT->getNumElements() == 2 &&
552+
"TypedBufferLoad double vector has wrong size");
553+
ExtractNum = 4;
554+
}
555+
549556
Type *Ty = VectorType::get(Builder.getInt32Ty(), ExtractNum, false);
550557

551558
Type *LoadType = StructType::get(Ty, Builder.getInt1Ty());
552-
auto *X =
559+
CallInst *Load =
553560
Builder.CreateIntrinsic(LoadType, Intrinsic::dx_resource_load_typedbuffer,
554561
{Orig->getOperand(0), Orig->getOperand(1)});
555562

556-
// create new extract value
557-
Value *Extract = Builder.CreateExtractValue(X, {0});
563+
// extract the buffer load's result
564+
Value *Extract = Builder.CreateExtractValue(Load, {0});
558565

559566
SmallVector<Value *> ExtractElements;
560567
for (unsigned I = 0; I < ExtractNum; ++I)
561568
ExtractElements.push_back(
562569
Builder.CreateExtractElement(Extract, (uint64_t)I));
563570

564571
// combine into double(s)
565-
Value *Result =
566-
PoisonValue::get(VectorType::get(Builder.getDoubleTy(), 2, false));
572+
Value *Result = PoisonValue::get(BufferTy);
567573
for (unsigned I = 0; I < ExtractNum; I += 2) {
568574
Value *Dbl =
569575
Builder.CreateIntrinsic(Builder.getDoubleTy(), Intrinsic::dx_asdouble,
@@ -574,26 +580,29 @@ static void expandTypedBufferLoadIntrinsic(CallInst *Orig) {
574580
Result = Dbl;
575581
}
576582

577-
assert(Orig->hasOneUser() && "TypedBufferLoad is expected to have one user");
578-
auto *U = Orig->user_back();
579-
auto *OldExtract = dyn_cast<ExtractValueInst>(U);
580-
if (!OldExtract)
581-
llvm_unreachable("TypedBufferLoad's only users should be ExtractValueInst");
582-
OldExtract->replaceAllUsesWith(Result);
583-
OldExtract->eraseFromParent();
583+
Value *CheckBit = Builder.CreateExtractValue(Load, {1});
584+
585+
Value *Struct = PoisonValue::get(Orig->getType());
586+
Struct = Builder.CreateInsertValue(Struct, Result, {0});
587+
Struct = Builder.CreateInsertValue(Struct, CheckBit, {1});
588+
return Struct;
584589
}
585590

586-
void expandTypedBufferStoreIntrinsic(CallInst *Orig) {
591+
static Value *expandTypedBufferStoreIntrinsic(CallInst *Orig) {
587592
IRBuilder<> Builder(Orig);
588593

589-
unsigned ExtractNum =
590-
Orig->getFunctionType()->getParamType(2)->isVectorTy() ? 4 : 2;
594+
Type *BufferTy = Orig->getFunctionType()->getParamType(2);
595+
596+
unsigned ExtractNum = 2;
597+
if (auto *VT = dyn_cast<FixedVectorType>(BufferTy)) {
598+
assert(VT->getNumElements() == 2 &&
599+
"TypedBufferStore double vector has wrong size");
600+
ExtractNum = 4;
601+
}
602+
591603
Type *SplitElementTy = Builder.getInt32Ty();
592-
SmallVector<int> Mask = {0, 1};
593-
if (ExtractNum == 4) {
604+
if (ExtractNum == 4)
594605
SplitElementTy = VectorType::get(SplitElementTy, 2, false);
595-
Mask = {0, 2, 1, 3};
596-
}
597606

598607
// split our double(s)
599608
auto *SplitTy = llvm::StructType::get(SplitElementTy, SplitElementTy);
@@ -610,9 +619,9 @@ void expandTypedBufferStoreIntrinsic(CallInst *Orig) {
610619
} else
611620
Val = Builder.CreateShuffleVector(LowBits, HighBits, {0, 2, 1, 3});
612621

613-
Builder.CreateIntrinsic(Builder.getVoidTy(),
614-
Intrinsic::dx_resource_store_typedbuffer,
615-
{Orig->getOperand(0), Orig->getOperand(1), Val});
622+
return Builder.CreateIntrinsic(
623+
Builder.getVoidTy(), Intrinsic::dx_resource_store_typedbuffer,
624+
{Orig->getOperand(0), Orig->getOperand(1), Val});
616625
}
617626

618627
static Intrinsic::ID getMaxForClamp(Intrinsic::ID ClampIntrinsic) {
@@ -744,13 +753,11 @@ static bool expandIntrinsic(Function &F, CallInst *Orig) {
744753
Result = expandRadiansIntrinsic(Orig);
745754
break;
746755
case Intrinsic::dx_resource_load_typedbuffer:
747-
expandTypedBufferLoadIntrinsic(Orig);
748-
Orig->eraseFromParent();
749-
return true;
756+
Result = expandTypedBufferLoadIntrinsic(Orig);
757+
break;
750758
case Intrinsic::dx_resource_store_typedbuffer:
751-
expandTypedBufferStoreIntrinsic(Orig);
752-
Orig->eraseFromParent();
753-
return true;
759+
Result = expandTypedBufferStoreIntrinsic(Orig);
760+
break;
754761
case Intrinsic::usub_sat:
755762
Result = expandUsubSat(Orig);
756763
break;

‎llvm/test/CodeGen/DirectX/BufferLoadDouble.ll‎

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,10 @@ define void @loadf64() {
1111
@llvm.dx.resource.handlefrombinding.tdx.TypedBuffer_f64_1_0_0t(
1212
i32 0, i32 1, i32 1, i32 0, i1 false)
1313

14+
ret void
15+
1416
; check we load an <2 x i32> instead of a double
17+
; CHECK-NOT: call {double, i1} @llvm.dx.resource.load.typedbuffer
1518
; CHECK: [[L0:%.*]] = call { <2 x i32>, i1 }
1619
; CHECK-SAME: @llvm.dx.resource.load.typedbuffer.v2i32.tdx.TypedBuffer_f64_1_0_0t(
1720
; CHECK-SAME: target("dx.TypedBuffer", double, 1, 0, 0) [[B]], i32 0)
@@ -22,7 +25,12 @@ define void @loadf64() {
2225
; CHECK: [[D0:%.*]] = extractvalue { <2 x i32>, i1 } [[L0]], 0
2326
; CHECK: [[Lo:%.*]] = extractelement <2 x i32> [[D0]], i64 0
2427
; CHECK: [[Hi:%.*]] = extractelement <2 x i32> [[D0]], i64 1
25-
; CHECK: call double @llvm.dx.asdouble.i32(i32 [[Lo]], i32 [[Hi]])
28+
; CHECK: [[DBL:%.*]] = call double @llvm.dx.asdouble.i32(i32 [[Lo]], i32 [[Hi]])
29+
; construct a new {double, i1}
30+
; CHECK: [[CB:%.*]] = extractvalue { <2 x i32>, i1 } [[L0]], 1
31+
; CHECK: [[S1:%.*]] = insertvalue { double, i1 } poison, double [[DBL]], 0
32+
; CHECK: [[S2:%.*]] = insertvalue { double, i1 } [[S1]], i1 [[CB]], 1
33+
; CHECK: extractvalue { double, i1 } [[S2]], 0
2634
%data0 = extractvalue {double, i1} %load0, 0
2735
ret void
2836
}
@@ -52,7 +60,48 @@ define void @loadv2f64() {
5260
; CHECK: [[Dbl1:%.*]] = call double @llvm.dx.asdouble.i32(i32 [[Lo1]], i32 [[Hi1]])
5361
; CHECK: [[Vec:%.*]] = insertelement <2 x double> poison, double [[Dbl1]], i64 0
5462
; CHECK: [[Dbl2:%.*]] = call double @llvm.dx.asdouble.i32(i32 [[Lo2]], i32 [[Hi2]])
55-
; CHECK: insertelement <2 x double> [[Vec]], double [[Dbl2]], i64 1
63+
; CHECK: [[Vec2:%.*]] = insertelement <2 x double> [[Vec]], double [[Dbl2]], i64 1
64+
; construct a new {<2 x double>, i1}
65+
; CHECK: [[CB:%.*]] = extractvalue { <4 x i32>, i1 } [[L0]], 1
66+
; CHECK: [[S1:%.*]] = insertvalue { <2 x double>, i1 } poison, <2 x double> [[Vec2]], 0
67+
; CHECK: [[S2:%.*]] = insertvalue { <2 x double>, i1 } [[S1]], i1 [[CB]], 1
68+
; CHECK: extractvalue { <2 x double>, i1 } [[S2]], 0
5669
%data0 = extractvalue { <2 x double>, i1 } %load0, 0
5770
ret void
5871
}
72+
73+
; show we properly handle extracting the check bit
74+
define void @loadf64WithCheckBit() {
75+
; check the handle from binding is unchanged
76+
; CHECK: [[B:%.*]] = call target("dx.TypedBuffer", double, 1, 0, 0)
77+
; CHECK-SAME: @llvm.dx.resource.handlefrombinding.tdx.TypedBuffer_f64_1_0_0t(
78+
; CHECK-SAME: i32 0, i32 1, i32 1, i32 0, i1 false)
79+
%buffer = call target("dx.TypedBuffer", double, 1, 0, 0)
80+
@llvm.dx.resource.handlefrombinding.tdx.TypedBuffer_f64_1_0_0t(
81+
i32 0, i32 1, i32 1, i32 0, i1 false)
82+
83+
ret void
84+
85+
; check we load an <2 x i32> instead of a double
86+
; CHECK-NOT: call {double, i1} @llvm.dx.resource.load.typedbuffer
87+
; CHECK: [[L0:%.*]] = call { <2 x i32>, i1 }
88+
; CHECK-SAME: @llvm.dx.resource.load.typedbuffer.v2i32.tdx.TypedBuffer_f64_1_0_0t(
89+
; CHECK-SAME: target("dx.TypedBuffer", double, 1, 0, 0) [[B]], i32 0)
90+
%load0 = call {double, i1} @llvm.dx.resource.load.typedbuffer(
91+
target("dx.TypedBuffer", double, 1, 0, 0) %buffer, i32 0)
92+
93+
; check we extract the two i32 and construct a double
94+
; CHECK: [[D0:%.*]] = extractvalue { <2 x i32>, i1 } [[L0]], 0
95+
; CHECK: [[Lo:%.*]] = extractelement <2 x i32> [[D0]], i64 0
96+
; CHECK: [[Hi:%.*]] = extractelement <2 x i32> [[D0]], i64 1
97+
; CHECK: [[DBL:%.*]] = call double @llvm.dx.asdouble.i32(i32 [[Lo]], i32 [[Hi]])
98+
; construct a new {double, i1}
99+
; CHECK: [[CB:%.*]] = extractvalue { <2 x i32>, i1 } [[L0]], 1
100+
; CHECK: [[S1:%.*]] = insertvalue { double, i1 } poison, double [[DBL]], 0
101+
; CHECK: [[S2:%.*]] = insertvalue { double, i1 } [[S1]], i1 [[CB]], 1
102+
; CHECK: extractvalue { double, i1 } [[S2]], 0
103+
%data0 = extractvalue {double, i1} %load0, 0
104+
; CHECK: extractvalue { double, i1 } [[S2]], 1
105+
%cb = extractvalue {double, i1} %load0, 1
106+
ret void
107+
}

0 commit comments

Comments
 (0)