Skip to content

Commit 89d1255

Browse files
Bit width of input/result types in OpSConvert/OpUConvert must not be the same (#89737)
This PR fixes the issue #88908 Attached test case is updated to check that OpSConvert/OpUConvert is not generated when input and result types are identical.
1 parent 282b56f commit 89d1255

File tree

3 files changed

+41
-14
lines changed

3 files changed

+41
-14
lines changed

llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1587,8 +1587,18 @@ bool SPIRVInstructionSelector::selectIToF(Register ResVReg,
15871587
bool SPIRVInstructionSelector::selectExt(Register ResVReg,
15881588
const SPIRVType *ResType,
15891589
MachineInstr &I, bool IsSigned) const {
1590-
if (GR.isScalarOrVectorOfType(I.getOperand(1).getReg(), SPIRV::OpTypeBool))
1590+
Register SrcReg = I.getOperand(1).getReg();
1591+
if (GR.isScalarOrVectorOfType(SrcReg, SPIRV::OpTypeBool))
15911592
return selectSelect(ResVReg, ResType, I, IsSigned);
1593+
1594+
SPIRVType *SrcType = GR.getSPIRVTypeForVReg(SrcReg);
1595+
if (SrcType == ResType)
1596+
return BuildMI(*I.getParent(), I, I.getDebugLoc(),
1597+
TII.get(TargetOpcode::COPY))
1598+
.addDef(ResVReg)
1599+
.addUse(SrcReg)
1600+
.constrainAllUses(TII, TRI, RBI);
1601+
15921602
unsigned Opcode = IsSigned ? SPIRV::OpSConvert : SPIRV::OpUConvert;
15931603
return selectUnOp(ResVReg, ResType, I, Opcode);
15941604
}
@@ -1622,11 +1632,16 @@ bool SPIRVInstructionSelector::selectIntToBool(Register IntReg,
16221632
bool SPIRVInstructionSelector::selectTrunc(Register ResVReg,
16231633
const SPIRVType *ResType,
16241634
MachineInstr &I) const {
1625-
if (GR.isScalarOrVectorOfType(ResVReg, SPIRV::OpTypeBool)) {
1626-
Register IntReg = I.getOperand(1).getReg();
1627-
const SPIRVType *ArgType = GR.getSPIRVTypeForVReg(IntReg);
1635+
Register IntReg = I.getOperand(1).getReg();
1636+
const SPIRVType *ArgType = GR.getSPIRVTypeForVReg(IntReg);
1637+
if (GR.isScalarOrVectorOfType(ResVReg, SPIRV::OpTypeBool))
16281638
return selectIntToBool(IntReg, ResVReg, I, ArgType, ResType);
1629-
}
1639+
if (ArgType == ResType)
1640+
return BuildMI(*I.getParent(), I, I.getDebugLoc(),
1641+
TII.get(TargetOpcode::COPY))
1642+
.addDef(ResVReg)
1643+
.addUse(IntReg)
1644+
.constrainAllUses(TII, TRI, RBI);
16301645
bool IsSigned = GR.isScalarOrVectorSigned(ResType);
16311646
unsigned Opcode = IsSigned ? SPIRV::OpSConvert : SPIRV::OpUConvert;
16321647
return selectUnOp(ResVReg, ResType, I, Opcode);

llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,7 @@ createNewIdReg(SPIRVType *SpvType, Register SrcReg, MachineRegisterInfo &MRI,
252252
if (!SpvType)
253253
SpvType = GR.getSPIRVTypeForVReg(SrcReg);
254254
assert(SpvType && "VReg is expected to have SPIRV type");
255+
LLT SrcLLT = MRI.getType(SrcReg);
255256
LLT NewT = LLT::scalar(32);
256257
bool IsFloat = SpvType->getOpcode() == SPIRV::OpTypeFloat;
257258
bool IsVectorFloat =
@@ -261,10 +262,10 @@ createNewIdReg(SPIRVType *SpvType, Register SrcReg, MachineRegisterInfo &MRI,
261262
IsFloat |= IsVectorFloat;
262263
auto GetIdOp = IsFloat ? SPIRV::GET_fID : SPIRV::GET_ID;
263264
auto DstClass = IsFloat ? &SPIRV::fIDRegClass : &SPIRV::IDRegClass;
264-
if (MRI.getType(SrcReg).isPointer()) {
265+
if (SrcLLT.isPointer()) {
265266
unsigned PtrSz = GR.getPointerSize();
266267
NewT = LLT::pointer(0, PtrSz);
267-
bool IsVec = MRI.getType(SrcReg).isVector();
268+
bool IsVec = SrcLLT.isVector();
268269
if (IsVec)
269270
NewT = LLT::fixed_vector(2, NewT);
270271
if (PtrSz == 64) {
@@ -284,7 +285,7 @@ createNewIdReg(SPIRVType *SpvType, Register SrcReg, MachineRegisterInfo &MRI,
284285
DstClass = &SPIRV::pID32RegClass;
285286
}
286287
}
287-
} else if (MRI.getType(SrcReg).isVector()) {
288+
} else if (SrcLLT.isVector()) {
288289
NewT = LLT::fixed_vector(2, NewT);
289290
if (IsFloat) {
290291
GetIdOp = SPIRV::GET_vfID;
@@ -483,7 +484,8 @@ static void processInstrsWithTypeFolding(MachineFunction &MF,
483484
continue;
484485
Register DstReg = MI.getOperand(0).getReg();
485486
bool IsDstPtr = MRI.getType(DstReg).isPointer();
486-
if (IsDstPtr || MRI.getType(DstReg).isVector())
487+
bool isDstVec = MRI.getType(DstReg).isVector();
488+
if (IsDstPtr || isDstVec)
487489
MRI.setRegClass(DstReg, &SPIRV::IDRegClass);
488490
// Don't need to reset type of register holding constant and used in
489491
// G_ADDRSPACE_CAST, since it breaks legalizer.

llvm/test/CodeGen/SPIRV/transcoding/memcpy-zext.ll

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,7 @@
33
; RUN: llc -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s --check-prefixes=CHECK,CHECK-64
44
; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %}
55

6-
; CHECK-64-DAG: %[[#i64:]] = OpTypeInt 64 0
7-
6+
; CHECK-DAG: %[[#i64:]] = OpTypeInt 64 0
87
; CHECK-DAG: %[[#i8:]] = OpTypeInt 8 0
98
; CHECK-DAG: %[[#i32:]] = OpTypeInt 32 0
109
; CHECK-DAG: %[[#one:]] = OpConstant %[[#i32]] 1
@@ -14,19 +13,28 @@
1413
; CHECK-DAG: %[[#test_arr_init:]] = OpConstantComposite %[[#i32x3]] %[[#one]] %[[#two]] %[[#three]]
1514
; CHECK-DAG: %[[#szconst1024:]] = OpConstant %[[#i32]] 1024
1615
; CHECK-DAG: %[[#szconst42:]] = OpConstant %[[#i8]] 42
16+
; CHECK-DAG: %[[#szconst123:]] = OpConstant %[[#i64]] 123
1717
; CHECK-DAG: %[[#const_i32x3_ptr:]] = OpTypePointer UniformConstant %[[#i32x3]]
1818
; CHECK-DAG: %[[#test_arr:]] = OpVariable %[[#const_i32x3_ptr]] UniformConstant %[[#test_arr_init]]
1919
; CHECK-DAG: %[[#i32x3_ptr:]] = OpTypePointer Function %[[#i32x3]]
2020
; CHECK: %[[#arr:]] = OpVariable %[[#i32x3_ptr]] Function
2121

2222
; CHECK-32: OpCopyMemorySized %[[#arr]] %[[#test_arr]] %[[#szconst1024]]
23-
; CHECK-64: %[[#szconstext1024:]] = OpUConvert %[[#i64:]] %[[#szconst1024:]]
24-
; CHECK-64: OpCopyMemorySized %[[#arr]] %[[#test_arr]] %[[#szconstext1024]]
25-
2623
; CHECK-32: %[[#szconstext42:]] = OpUConvert %[[#i32:]] %[[#szconst42:]]
2724
; CHECK-32: OpCopyMemorySized %[[#arr]] %[[#test_arr]] %[[#szconstext42]]
25+
; CHECK-32: OpCopyMemorySized %[[#arr]] %[[#test_arr]] %[[#szconst123]]
26+
27+
; If/when Backend stoped rewrite actual reg size of i8/i16/i32/i64 with i32,
28+
; i32 = G_TRUNC i64 would appear for the 32-bit target, switching the following
29+
; TODO patterns instead of the last line above.
30+
; TODO: %[[#szconstext123:]] = OpUConvert %[[#i32:]] %[[#szconst123:]]
31+
; TODO: OpCopyMemorySized %[[#arr]] %[[#test_arr]] %[[#szconst123]]
32+
33+
; CHECK-64: %[[#szconstext1024:]] = OpUConvert %[[#i64:]] %[[#szconst1024:]]
34+
; CHECK-64: OpCopyMemorySized %[[#arr]] %[[#test_arr]] %[[#szconstext1024]]
2835
; CHECK-64: %[[#szconstext42:]] = OpUConvert %[[#i64:]] %[[#szconst42:]]
2936
; CHECK-64: OpCopyMemorySized %[[#arr]] %[[#test_arr]] %[[#szconstext42]]
37+
; CHECK-64: OpCopyMemorySized %[[#arr]] %[[#test_arr]] %[[#szconst123]]
3038

3139
@__const.test.arr = private unnamed_addr addrspace(2) constant [3 x i32] [i32 1, i32 2, i32 3]
3240

@@ -36,8 +44,10 @@ entry:
3644
%dest = bitcast ptr %arr to ptr
3745
call void @llvm.memcpy.p0.p2.i32(ptr align 4 %dest, ptr addrspace(2) align 4 @__const.test.arr, i32 1024, i1 false)
3846
call void @llvm.memcpy.p0.p2.i8(ptr align 4 %dest, ptr addrspace(2) align 4 @__const.test.arr, i8 42, i1 false)
47+
call void @llvm.memcpy.p0.p2.i64(ptr align 4 %dest, ptr addrspace(2) align 4 @__const.test.arr, i64 123, i1 false)
3948
ret void
4049
}
4150

4251
declare void @llvm.memcpy.p0.p2.i32(ptr nocapture writeonly, ptr addrspace(2) nocapture readonly, i32, i1)
4352
declare void @llvm.memcpy.p0.p2.i8(ptr nocapture writeonly, ptr addrspace(2) nocapture readonly, i8, i1)
53+
declare void @llvm.memcpy.p0.p2.i64(ptr nocapture writeonly, ptr addrspace(2) nocapture readonly, i64, i1)

0 commit comments

Comments
 (0)