Bit width of input/result types in OpSConvert/OpUConvert must not be the same

[VyacheslavLevytskyy]

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.

[llvmbot]

@llvm/pr-subscribers-backend-spir-v

Author: Vyacheslav Levytskyy (VyacheslavLevytskyy)

Changes

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.

---

Full diff: https://github.com/llvm/llvm-project/pull/89737.diff

3 Files Affected:

• (modified) llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp (+20-5)
• (modified) llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp (+6-4)
• (modified) llvm/test/CodeGen/SPIRV/transcoding/memcpy-zext.ll (+15-5)

diff --git a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
index 21a69fc3ad9b44..30ebcc7e8364ae 100644
--- a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
@@ -1587,8 +1587,18 @@ bool SPIRVInstructionSelector::selectIToF(Register ResVReg,
 bool SPIRVInstructionSelector::selectExt(Register ResVReg,
                                          const SPIRVType *ResType,
                                          MachineInstr &I, bool IsSigned) const {
-  if (GR.isScalarOrVectorOfType(I.getOperand(1).getReg(), SPIRV::OpTypeBool))
+  Register SrcReg = I.getOperand(1).getReg();
+  if (GR.isScalarOrVectorOfType(SrcReg, SPIRV::OpTypeBool))
     return selectSelect(ResVReg, ResType, I, IsSigned);
+
+  SPIRVType *SrcType = GR.getSPIRVTypeForVReg(SrcReg);
+  if (SrcType == ResType)
+    return BuildMI(*I.getParent(), I, I.getDebugLoc(),
+                   TII.get(TargetOpcode::COPY))
+        .addDef(ResVReg)
+        .addUse(SrcReg)
+        .constrainAllUses(TII, TRI, RBI);
+
   unsigned Opcode = IsSigned ? SPIRV::OpSConvert : SPIRV::OpUConvert;
   return selectUnOp(ResVReg, ResType, I, Opcode);
 }
@@ -1622,11 +1632,16 @@ bool SPIRVInstructionSelector::selectIntToBool(Register IntReg,
 bool SPIRVInstructionSelector::selectTrunc(Register ResVReg,
                                            const SPIRVType *ResType,
                                            MachineInstr &I) const {
-  if (GR.isScalarOrVectorOfType(ResVReg, SPIRV::OpTypeBool)) {
-    Register IntReg = I.getOperand(1).getReg();
-    const SPIRVType *ArgType = GR.getSPIRVTypeForVReg(IntReg);
+  Register IntReg = I.getOperand(1).getReg();
+  const SPIRVType *ArgType = GR.getSPIRVTypeForVReg(IntReg);
+  if (GR.isScalarOrVectorOfType(ResVReg, SPIRV::OpTypeBool))
     return selectIntToBool(IntReg, ResVReg, I, ArgType, ResType);
-  }
+  if (ArgType == ResType)
+    return BuildMI(*I.getParent(), I, I.getDebugLoc(),
+                   TII.get(TargetOpcode::COPY))
+        .addDef(ResVReg)
+        .addUse(IntReg)
+        .constrainAllUses(TII, TRI, RBI);
   bool IsSigned = GR.isScalarOrVectorSigned(ResType);
   unsigned Opcode = IsSigned ? SPIRV::OpSConvert : SPIRV::OpUConvert;
   return selectUnOp(ResVReg, ResType, I, Opcode);
diff --git a/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp b/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp
index d16f6d5bf67ef4..466e72006a9bd1 100644
--- a/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp
@@ -252,6 +252,7 @@ createNewIdReg(SPIRVType *SpvType, Register SrcReg, MachineRegisterInfo &MRI,
   if (!SpvType)
     SpvType = GR.getSPIRVTypeForVReg(SrcReg);
   assert(SpvType && "VReg is expected to have SPIRV type");
+  LLT SrcLLT = MRI.getType(SrcReg);
   LLT NewT = LLT::scalar(32);
   bool IsFloat = SpvType->getOpcode() == SPIRV::OpTypeFloat;
   bool IsVectorFloat =
@@ -261,10 +262,10 @@ createNewIdReg(SPIRVType *SpvType, Register SrcReg, MachineRegisterInfo &MRI,
   IsFloat |= IsVectorFloat;
   auto GetIdOp = IsFloat ? SPIRV::GET_fID : SPIRV::GET_ID;
   auto DstClass = IsFloat ? &SPIRV::fIDRegClass : &SPIRV::IDRegClass;
-  if (MRI.getType(SrcReg).isPointer()) {
+  if (SrcLLT.isPointer()) {
     unsigned PtrSz = GR.getPointerSize();
     NewT = LLT::pointer(0, PtrSz);
-    bool IsVec = MRI.getType(SrcReg).isVector();
+    bool IsVec = SrcLLT.isVector();
     if (IsVec)
       NewT = LLT::fixed_vector(2, NewT);
     if (PtrSz == 64) {
@@ -284,7 +285,7 @@ createNewIdReg(SPIRVType *SpvType, Register SrcReg, MachineRegisterInfo &MRI,
         DstClass = &SPIRV::pID32RegClass;
       }
     }
-  } else if (MRI.getType(SrcReg).isVector()) {
+  } else if (SrcLLT.isVector()) {
     NewT = LLT::fixed_vector(2, NewT);
     if (IsFloat) {
       GetIdOp = SPIRV::GET_vfID;
@@ -483,7 +484,8 @@ static void processInstrsWithTypeFolding(MachineFunction &MF,
         continue;
       Register DstReg = MI.getOperand(0).getReg();
       bool IsDstPtr = MRI.getType(DstReg).isPointer();
-      if (IsDstPtr || MRI.getType(DstReg).isVector())
+      bool isDstVec = MRI.getType(DstReg).isVector();
+      if (IsDstPtr || isDstVec)
         MRI.setRegClass(DstReg, &SPIRV::IDRegClass);
       // Don't need to reset type of register holding constant and used in
       // G_ADDRSPACE_CAST, since it breaks legalizer.
diff --git a/llvm/test/CodeGen/SPIRV/transcoding/memcpy-zext.ll b/llvm/test/CodeGen/SPIRV/transcoding/memcpy-zext.ll
index ea0197548a8154..89fa93b4fcda1c 100644
--- a/llvm/test/CodeGen/SPIRV/transcoding/memcpy-zext.ll
+++ b/llvm/test/CodeGen/SPIRV/transcoding/memcpy-zext.ll
@@ -3,8 +3,7 @@
 ; RUN: llc -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s --check-prefixes=CHECK,CHECK-64
 ; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %}
 
-; CHECK-64-DAG: %[[#i64:]] = OpTypeInt 64 0
-
+; CHECK-DAG:    %[[#i64:]] = OpTypeInt 64 0
 ; CHECK-DAG:    %[[#i8:]] = OpTypeInt 8 0
 ; CHECK-DAG:    %[[#i32:]] = OpTypeInt 32 0
 ; CHECK-DAG:    %[[#one:]] = OpConstant %[[#i32]] 1
@@ -14,19 +13,28 @@
 ; CHECK-DAG:    %[[#test_arr_init:]] = OpConstantComposite %[[#i32x3]] %[[#one]] %[[#two]] %[[#three]]
 ; CHECK-DAG:    %[[#szconst1024:]] = OpConstant %[[#i32]] 1024
 ; CHECK-DAG:    %[[#szconst42:]] = OpConstant %[[#i8]] 42
+; CHECK-DAG:    %[[#szconst123:]] = OpConstant %[[#i64]] 123
 ; CHECK-DAG:    %[[#const_i32x3_ptr:]] = OpTypePointer UniformConstant %[[#i32x3]]
 ; CHECK-DAG:    %[[#test_arr:]] = OpVariable %[[#const_i32x3_ptr]] UniformConstant %[[#test_arr_init]]
 ; CHECK-DAG:    %[[#i32x3_ptr:]] = OpTypePointer Function %[[#i32x3]]
 ; CHECK:        %[[#arr:]] = OpVariable %[[#i32x3_ptr]] Function
 
 ; CHECK-32:     OpCopyMemorySized %[[#arr]] %[[#test_arr]] %[[#szconst1024]]
-; CHECK-64:     %[[#szconstext1024:]] = OpUConvert %[[#i64:]] %[[#szconst1024:]]
-; CHECK-64:     OpCopyMemorySized %[[#arr]] %[[#test_arr]] %[[#szconstext1024]]
-
 ; CHECK-32:     %[[#szconstext42:]] = OpUConvert %[[#i32:]] %[[#szconst42:]]
 ; CHECK-32:     OpCopyMemorySized %[[#arr]] %[[#test_arr]] %[[#szconstext42]]
+; CHECK-32:     OpCopyMemorySized %[[#arr]] %[[#test_arr]] %[[#szconst123]]
+
+; If/when Backend stoped rewrite actual reg size of i8/i16/i32/i64 with i32,
+; i32 = G_TRUNC i64 would appear for the 32-bit target, switching the following
+; TODO patterns instead of the last line above.
+; TODO:         %[[#szconstext123:]] = OpUConvert %[[#i32:]] %[[#szconst123:]]
+; TODO:         OpCopyMemorySized %[[#arr]] %[[#test_arr]] %[[#szconst123]]
+
+; CHECK-64:     %[[#szconstext1024:]] = OpUConvert %[[#i64:]] %[[#szconst1024:]]
+; CHECK-64:     OpCopyMemorySized %[[#arr]] %[[#test_arr]] %[[#szconstext1024]]
 ; CHECK-64:     %[[#szconstext42:]] = OpUConvert %[[#i64:]] %[[#szconst42:]]
 ; CHECK-64:     OpCopyMemorySized %[[#arr]] %[[#test_arr]] %[[#szconstext42]]
+; CHECK-64:     OpCopyMemorySized %[[#arr]] %[[#test_arr]] %[[#szconst123]]
 
 @__const.test.arr = private unnamed_addr addrspace(2) constant [3 x i32] [i32 1, i32 2, i32 3]
 
@@ -36,8 +44,10 @@ entry:
   %dest = bitcast ptr %arr to ptr
   call void @llvm.memcpy.p0.p2.i32(ptr align 4 %dest, ptr addrspace(2) align 4 @__const.test.arr, i32 1024, i1 false)
   call void @llvm.memcpy.p0.p2.i8(ptr align 4 %dest, ptr addrspace(2) align 4 @__const.test.arr, i8 42, i1 false)
+  call void @llvm.memcpy.p0.p2.i64(ptr align 4 %dest, ptr addrspace(2) align 4 @__const.test.arr, i64 123, i1 false)
   ret void
 }
 
 declare void @llvm.memcpy.p0.p2.i32(ptr nocapture writeonly, ptr addrspace(2) nocapture readonly, i32, i1)
 declare void @llvm.memcpy.p0.p2.i8(ptr nocapture writeonly, ptr addrspace(2) nocapture readonly, i8, i1)
+declare void @llvm.memcpy.p0.p2.i64(ptr nocapture writeonly, ptr addrspace(2) nocapture readonly, i64, i1)