Skip to content

Commit 95bb7ef

Browse files
AlexMacleanAlexMaclean
committed
address comments
1 parent 5a5c1a2 commit 95bb7ef

File tree

3 files changed

+63
-50
lines changed

3 files changed

+63
-50
lines changed

‎llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp

Lines changed: 48 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1389,26 +1389,6 @@ SDValue NVPTXTargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
13891389
return DAG.getConstant(I, dl, MVT::i32);
13901390
};
13911391

1392-
// Variadic arguments.
1393-
//
1394-
// Normally, for each argument, we declare a param scalar or a param
1395-
// byte array in the .param space, and store the argument value to that
1396-
// param scalar or array starting at offset 0.
1397-
//
1398-
// In the case of the first variadic argument, we declare a vararg byte array
1399-
// with size 0. The exact size of this array isn't known at this point, so
1400-
// it'll be patched later. All the variadic arguments will be stored to this
1401-
// array at a certain offset (which gets tracked by 'VAOffset'). The offset is
1402-
// initially set to 0, so it can be used for non-variadic arguments (which use
1403-
// 0 offset) to simplify the code.
1404-
//
1405-
// After all vararg is processed, 'VAOffset' holds the size of the
1406-
// vararg byte array.
1407-
1408-
SDValue VADeclareParam = SDValue(); // vararg byte array
1409-
const unsigned FirstVAArg = CLI.NumFixedArgs; // position of first variadic
1410-
unsigned VAOffset = 0; // current offset in the param array
1411-
14121392
const unsigned UniqueCallSite = GlobalUniqueCallSite++;
14131393
const SDValue CallChain = CLI.Chain;
14141394
const SDValue StartChain =
@@ -1417,7 +1397,7 @@ SDValue NVPTXTargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
14171397

14181398
SmallVector<SDValue, 16> CallPrereqs{StartChain};
14191399

1420-
const auto DeclareScalarParam = [&](SDValue Symbol, unsigned Size) {
1400+
const auto MakeDeclareScalarParam = [&](SDValue Symbol, unsigned Size) {
14211401
// PTX ABI requires integral types to be at least 32 bits in size. FP16 is
14221402
// loaded/stored using i16, so it's handled here as well.
14231403
const unsigned SizeBits = promoteScalarArgumentSize(Size * 8);
@@ -1429,8 +1409,8 @@ SDValue NVPTXTargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
14291409
return Declare;
14301410
};
14311411

1432-
const auto DeclareArrayParam = [&](SDValue Symbol, Align Align,
1433-
unsigned Size) {
1412+
const auto MakeDeclareArrayParam = [&](SDValue Symbol, Align Align,
1413+
unsigned Size) {
14341414
SDValue Declare = DAG.getNode(
14351415
NVPTXISD::DeclareArrayParam, dl, {MVT::Other, MVT::Glue},
14361416
{StartChain, Symbol, GetI32(Align.value()), GetI32(Size), DeclareGlue});
@@ -1439,6 +1419,33 @@ SDValue NVPTXTargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
14391419
return Declare;
14401420
};
14411421

1422+
// Variadic arguments.
1423+
//
1424+
// Normally, for each argument, we declare a param scalar or a param
1425+
// byte array in the .param space, and store the argument value to that
1426+
// param scalar or array starting at offset 0.
1427+
//
1428+
// In the case of the first variadic argument, we declare a vararg byte array
1429+
// with size 0. The exact size of this array isn't known at this point, so
1430+
// it'll be patched later. All the variadic arguments will be stored to this
1431+
// array at a certain offset (which gets tracked by 'VAOffset'). The offset is
1432+
// initially set to 0, so it can be used for non-variadic arguments (which use
1433+
// 0 offset) to simplify the code.
1434+
//
1435+
// After all vararg is processed, 'VAOffset' holds the size of the
1436+
// vararg byte array.
1437+
assert((CLI.IsVarArg || CLI.Args.size() == CLI.NumFixedArgs) &&
1438+
"Non-VarArg function with extra arguments");
1439+
1440+
const unsigned FirstVAArg = CLI.NumFixedArgs; // position of first variadic
1441+
unsigned VAOffset = 0; // current offset in the param array
1442+
1443+
const SDValue VADeclareParam =
1444+
CLI.Args.size() > FirstVAArg
1445+
? MakeDeclareArrayParam(getCallParamSymbol(DAG, FirstVAArg, MVT::i32),
1446+
Align(STI.getMaxRequiredAlignment()), 0)
1447+
: SDValue();
1448+
14421449
// Args.size() and Outs.size() need not match.
14431450
// Outs.size() will be larger
14441451
// * if there is an aggregate argument with multiple fields (each field
@@ -1499,21 +1506,17 @@ SDValue NVPTXTargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
14991506
"type size mismatch");
15001507

15011508
const SDValue ArgDeclare = [&]() {
1502-
if (IsVAArg) {
1503-
if (ArgI == FirstVAArg)
1504-
VADeclareParam = DeclareArrayParam(
1505-
ParamSymbol, Align(STI.getMaxRequiredAlignment()), 0);
1509+
if (IsVAArg)
15061510
return VADeclareParam;
1507-
}
15081511

15091512
if (IsByVal || shouldPassAsArray(Arg.Ty))
1510-
return DeclareArrayParam(ParamSymbol, ArgAlign, TypeSize);
1513+
return MakeDeclareArrayParam(ParamSymbol, ArgAlign, TypeSize);
15111514

15121515
assert(ArgOuts.size() == 1 && "We must pass only one value as non-array");
15131516
assert((ArgOuts[0].VT.isInteger() || ArgOuts[0].VT.isFloatingPoint()) &&
15141517
"Only int and float types are supported as non-array arguments");
15151518

1516-
return DeclareScalarParam(ParamSymbol, TypeSize);
1519+
return MakeDeclareScalarParam(ParamSymbol, TypeSize);
15171520
}();
15181521

15191522
// PTX Interoperability Guide 3.3(A): [Integer] Values shorter
@@ -1573,7 +1576,7 @@ SDValue NVPTXTargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
15731576
if (NumElts == 1) {
15741577
Val = GetStoredValue(J, EltVT, CurrentAlign);
15751578
} else {
1576-
SmallVector<SDValue, 6> StoreVals;
1579+
SmallVector<SDValue, 8> StoreVals;
15771580
for (const unsigned K : llvm::seq(NumElts)) {
15781581
SDValue ValJ = GetStoredValue(J + K, EltVT, CurrentAlign);
15791582
if (ValJ.getValueType().isVector())
@@ -1614,9 +1617,9 @@ SDValue NVPTXTargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
16141617
const unsigned ResultSize = DL.getTypeAllocSize(RetTy);
16151618
if (shouldPassAsArray(RetTy)) {
16161619
const Align RetAlign = getArgumentAlignment(CB, RetTy, 0, DL);
1617-
DeclareArrayParam(RetSymbol, RetAlign, ResultSize);
1620+
MakeDeclareArrayParam(RetSymbol, RetAlign, ResultSize);
16181621
} else {
1619-
DeclareScalarParam(RetSymbol, ResultSize);
1622+
MakeDeclareScalarParam(RetSymbol, ResultSize);
16201623
}
16211624
}
16221625

@@ -1740,17 +1743,16 @@ SDValue NVPTXTargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
17401743

17411744
LoadChains.push_back(R.getValue(1));
17421745

1743-
if (NumElts == 1) {
1746+
if (NumElts == 1)
17441747
ProxyRegOps.push_back(R);
1745-
} else {
1748+
else
17461749
for (const unsigned J : llvm::seq(NumElts)) {
17471750
SDValue Elt = DAG.getNode(
17481751
LoadVT.isVector() ? ISD::EXTRACT_SUBVECTOR
17491752
: ISD::EXTRACT_VECTOR_ELT,
17501753
dl, LoadVT, R, DAG.getVectorIdxConstant(J * PackingAmt, dl));
17511754
ProxyRegOps.push_back(Elt);
17521755
}
1753-
}
17541756
I += NumElts;
17551757
}
17561758
}
@@ -5770,7 +5772,7 @@ static SDValue sinkProxyReg(SDValue R, SDValue Chain,
57705772
{Chain, R});
57715773
}
57725774
case ISD::BUILD_VECTOR: {
5773-
if (DCI.isAfterLegalizeDAG())
5775+
if (DCI.isBeforeLegalize())
57745776
return SDValue();
57755777

57765778
SmallVector<SDValue, 16> Ops;
@@ -5782,6 +5784,15 @@ static SDValue sinkProxyReg(SDValue R, SDValue Chain,
57825784
}
57835785
return DCI.DAG.getNode(ISD::BUILD_VECTOR, SDLoc(R), R.getValueType(), Ops);
57845786
}
5787+
case ISD::EXTRACT_VECTOR_ELT: {
5788+
if (DCI.isBeforeLegalize())
5789+
return SDValue();
5790+
5791+
if (SDValue V = sinkProxyReg(R.getOperand(0), Chain, DCI))
5792+
return DCI.DAG.getNode(ISD::EXTRACT_VECTOR_ELT, SDLoc(R), R.getValueType(),
5793+
V, R.getOperand(1));
5794+
return SDValue();
5795+
}
57855796
default:
57865797
return SDValue();
57875798
}

‎llvm/test/CodeGen/NVPTX/convert-call-to-indirect.ll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,8 +173,8 @@ define i64 @test_param_type_mismatch_variadic(ptr %p) {
173173
; CHECK-NEXT: // %bb.0:
174174
; CHECK-NEXT: ld.param.b64 %rd1, [test_param_type_mismatch_variadic_param_0];
175175
; CHECK-NEXT: { // callseq 4, 0
176-
; CHECK-NEXT: .param .b64 param0;
177176
; CHECK-NEXT: .param .align 8 .b8 param1[8];
177+
; CHECK-NEXT: .param .b64 param0;
178178
; CHECK-NEXT: .param .b64 retval0;
179179
; CHECK-NEXT: st.param.b64 [param0], %rd1;
180180
; CHECK-NEXT: st.param.b64 [param1], 7;
@@ -195,8 +195,8 @@ define i64 @test_param_count_mismatch_variadic(ptr %p) {
195195
; CHECK-NEXT: // %bb.0:
196196
; CHECK-NEXT: ld.param.b64 %rd1, [test_param_count_mismatch_variadic_param_0];
197197
; CHECK-NEXT: { // callseq 5, 0
198-
; CHECK-NEXT: .param .b64 param0;
199198
; CHECK-NEXT: .param .align 8 .b8 param1[8];
199+
; CHECK-NEXT: .param .b64 param0;
200200
; CHECK-NEXT: .param .b64 retval0;
201201
; CHECK-NEXT: st.param.b64 [param0], %rd1;
202202
; CHECK-NEXT: st.param.b64 [param1], 7;

‎llvm/test/CodeGen/NVPTX/param-load-store.ll

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ define <4 x i8> @test_v4i8(<4 x i8> %a) {
243243
; CHECK: call.uni (retval0), test_v5i8,
244244
; CHECK-DAG: ld.param.b32 [[RE0:%r[0-9]+]], [retval0];
245245
; CHECK-DAG: ld.param.b8 [[RE4:%rs[0-9]+]], [retval0+4];
246-
; CHECK-DAG: st.param.b32 [func_retval0], {{%r[0-9]+}};
246+
; CHECK-DAG: st.param.b32 [func_retval0], [[RE0]];
247247
; CHECK-DAG: st.param.b8 [func_retval0+4], [[RE4]];
248248
; CHECK-NEXT: ret;
249249
define <5 x i8> @test_v5i8(<5 x i8> %a) {
@@ -311,8 +311,9 @@ define signext i16 @test_i16s(i16 signext %a) {
311311
; CHECK-DAG: st.param.b32 [param0], [[E0]];
312312
; CHECK-DAG: st.param.b16 [param0+4], [[E2]];
313313
; CHECK: call.uni (retval0), test_v3i16,
314-
; CHECK: ld.param.v2.b16 {[[RE0:%rs[0-9]+]], [[RE1:%rs[0-9]+]]}, [retval0];
314+
; CHECK: ld.param.b32 [[RE:%r[0-9]+]], [retval0];
315315
; CHECK: ld.param.b16 [[RE2:%rs[0-9]+]], [retval0+4];
316+
; CHECK-DAG: mov.b32 {[[RE0:%rs[0-9]+]], [[RE1:%rs[0-9]+]]}, [[RE]];
316317
; CHECK-DAG: st.param.v2.b16 [func_retval0], {[[RE0]], [[RE1]]};
317318
; CHECK-DAG: st.param.b16 [func_retval0+4], [[RE2]];
318319
; CHECK-NEXT: ret;
@@ -347,9 +348,9 @@ define <4 x i16> @test_v4i16(<4 x i16> %a) {
347348
; CHECK-DAG: st.param.v2.b32 [param0], {[[E0]], [[E1]]};
348349
; CHECK-DAG: st.param.b16 [param0+8], [[E4]];
349350
; CHECK: call.uni (retval0), test_v5i16,
350-
; CHECK-DAG: ld.param.v4.b16 {[[RE0:%rs[0-9]+]], [[RE1:%rs[0-9]+]], [[RE2:%rs[0-9]+]], [[RE3:%rs[0-9]+]]}, [retval0];
351+
; CHECK-DAG: ld.param.v2.b32 {[[RE0:%r[0-9]+]], [[RE1:%r[0-9]+]]}, [retval0];
351352
; CHECK-DAG: ld.param.b16 [[RE4:%rs[0-9]+]], [retval0+8];
352-
; CHECK-DAG: st.param.v4.b16 [func_retval0], {[[RE0]], [[RE1]], [[RE2]], [[RE3]]}
353+
; CHECK-DAG: st.param.v2.b32 [func_retval0], {[[RE0]], [[RE1]]}
353354
; CHECK-DAG: st.param.b16 [func_retval0+8], [[RE4]];
354355
; CHECK-NEXT: ret;
355356
define <5 x i16> @test_v5i16(<5 x i16> %a) {
@@ -432,8 +433,9 @@ define <2 x bfloat> @test_v2bf16(<2 x bfloat> %a) {
432433
; CHECK-DAG: st.param.b32 [param0], [[E0]];
433434
; CHECK-DAG: st.param.b16 [param0+4], [[E2]];
434435
; CHECK: call.uni (retval0), test_v3f16,
435-
; CHECK-DAG: ld.param.v2.b16 {[[R0:%rs[0-9]+]], [[R1:%rs[0-9]+]]}, [retval0];
436+
; CHECK-DAG: ld.param.b32 [[R:%r[0-9]+]], [retval0];
436437
; CHECK-DAG: ld.param.b16 [[R2:%rs[0-9]+]], [retval0+4];
438+
; CHECK-DAG: mov.b32 {[[R0:%rs[0-9]+]], [[R1:%rs[0-9]+]]}, [[R]];
437439
; CHECK-DAG: st.param.v2.b16 [func_retval0], {[[R0]], [[R1]]};
438440
; CHECK-DAG: st.param.b16 [func_retval0+4], [[R2]];
439441
; CHECK: ret;
@@ -468,9 +470,9 @@ define <4 x half> @test_v4f16(<4 x half> %a) {
468470
; CHECK-DAG: st.param.v2.b32 [param0], {[[E0]], [[E1]]};
469471
; CHECK-DAG: st.param.b16 [param0+8], [[E4]];
470472
; CHECK: call.uni (retval0), test_v5f16,
471-
; CHECK-DAG: ld.param.v4.b16 {[[R0:%rs[0-9]+]], [[R1:%rs[0-9]+]], [[R2:%rs[0-9]+]], [[R3:%rs[0-9]+]]}, [retval0];
473+
; CHECK-DAG: ld.param.v2.b32 {[[R0:%r[0-9]+]], [[R1:%r[0-9]+]]}, [retval0];
472474
; CHECK-DAG: ld.param.b16 [[R4:%rs[0-9]+]], [retval0+8];
473-
; CHECK-DAG: st.param.v4.b16 [func_retval0], {[[R0]], [[R1]], [[R2]], [[R3]]};
475+
; CHECK-DAG: st.param.v2.b32 [func_retval0], {[[R0]], [[R1]]};
474476
; CHECK-DAG: st.param.b16 [func_retval0+8], [[R4]];
475477
; CHECK: ret;
476478
define <5 x half> @test_v5f16(<5 x half> %a) {
@@ -506,11 +508,11 @@ define <8 x half> @test_v8f16(<8 x half> %a) {
506508
; CHECK-DAG: st.param.v2.b32 [param0+8], {[[E2]], [[E3]]};
507509
; CHECK-DAG: st.param.b16 [param0+16], [[E8]];
508510
; CHECK: call.uni (retval0), test_v9f16,
509-
; CHECK-DAG: ld.param.v4.b16 {[[R0:%rs[0-9]+]], [[R1:%rs[0-9]+]], [[R2:%rs[0-9]+]], [[R3:%rs[0-9]+]]}, [retval0];
510-
; CHECK-DAG: ld.param.v4.b16 {[[R4:%rs[0-9]+]], [[R5:%rs[0-9]+]], [[R6:%rs[0-9]+]], [[R7:%rs[0-9]+]]}, [retval0+8];
511+
; CHECK-DAG: ld.param.v2.b32 {[[R0:%r[0-9]+]], [[R1:%r[0-9]+]]}, [retval0];
512+
; CHECK-DAG: ld.param.v2.b32 {[[R2:%r[0-9]+]], [[R3:%r[0-9]+]]}, [retval0+8];
511513
; CHECK-DAG: ld.param.b16 [[R8:%rs[0-9]+]], [retval0+16];
512-
; CHECK-DAG: st.param.v4.b16 [func_retval0], {[[R0]], [[R1]], [[R2]], [[R3]]};
513-
; CHECK-DAG: st.param.v4.b16 [func_retval0+8], {[[R4]], [[R5]], [[R6]], [[R7]]};
514+
; CHECK-DAG: st.param.v2.b32 [func_retval0], {[[R0]], [[R1]]};
515+
; CHECK-DAG: st.param.v2.b32 [func_retval0+8], {[[R2]], [[R3]]};
514516
; CHECK-DAG: st.param.b16 [func_retval0+16], [[R8]];
515517
; CHECK: ret;
516518
define <9 x half> @test_v9f16(<9 x half> %a) {

0 commit comments

Comments
 (0)