[AMDGPU] Perform uchar to float combine for ISD::SINT_TO_FP

[kzhuravl]

Summary

This will prevent following regression when enabling i16 support (D18049):
test/CodeGen/AMDGPU/cvt_f32_ubyte.ll

[• tstellarAMD]

lib/Target/AMDGPU/SIISelLowering.cpp

3560 ↗

(On Diff #75257)

I don't think this is correct, because it's replacing SINT_TO_FP with CVT_F32_UBYTE0, which is an unsigned conversion.

[• tstellarAMD]

lib/Target/AMDGPU/SIISelLowering.cpp

3525–3527 ↗

(On Diff #75257)

I think calling simplifyDemandedBits either here or in performUCharToFloat combine will eliminate the need for this code.

3560 ↗

(On Diff #75257)

Nevermind, I see that this transform is correct, because performUCharToFloatCombine() is checking that the high bits are all zero.

[kzhuravl]

Use getZExtOrTrunc

[kzhuravl]

lib/Target/AMDGPU/SIISelLowering.cpp

3525–3527 ↗

(On Diff #75257)

In this case, simplifyDemandedBits will replace zero_extend with any_extend.

[kzhuravl]

Only check for zero_extend

[• tstellarAMD]

LGTM. Can you mention in the commit message that this will prevent a regression when enabling i16 support, and note the test that would regress.

[arsenm]

lib/Target/AMDGPU/SIISelLowering.cpp

3560 ↗

(On Diff #75257)

That checks if the high 24 bits are zero. For the signed case it needs to check if 25 bits are 0 so it is still incorrect

llvm/trunk/lib/Target/AMDGPU/SIISelLowering.cpp

SITargetLowering::SITargetLowering(const TargetMachine &TM,
   setTargetDAGCombine(ISD::SMIN);
   setTargetDAGCombine(ISD::SMAX);
   setTargetDAGCombine(ISD::UMIN);
   setTargetDAGCombine(ISD::UMAX);
   setTargetDAGCombine(ISD::SETCC);
   setTargetDAGCombine(ISD::AND);
   setTargetDAGCombine(ISD::OR);
   setTargetDAGCombine(ISD::XOR);
+  setTargetDAGCombine(ISD::SINT_TO_FP);
   setTargetDAGCombine(ISD::UINT_TO_FP);
   setTargetDAGCombine(ISD::FCANONICALIZE);
 
   // All memory operations. Some folding on the pointer operand is done to help
   // matching the constant offsets in the addressing modes.
   setTargetDAGCombine(ISD::LOAD);
   setTargetDAGCombine(ISD::STORE);
   setTargetDAGCombine(ISD::ATOMIC_LOAD);
case AMDGPUISD::FMAX_LEGACY: {
     break;
   }
 
   case AMDGPUISD::CVT_F32_UBYTE0:
   case AMDGPUISD::CVT_F32_UBYTE1:
   case AMDGPUISD::CVT_F32_UBYTE2:
   case AMDGPUISD::CVT_F32_UBYTE3: {
     unsigned Offset = N->getOpcode() - AMDGPUISD::CVT_F32_UBYTE0;
+
     SDValue Src = N->getOperand(0);
+    SDValue Srl = N->getOperand(0);
+    if (Srl.getOpcode() == ISD::ZERO_EXTEND)
+      Srl = Srl.getOperand(0);
 
     // TODO: Handle (or x, (srl y, 8)) pattern when known bits are zero.
-    if (Src.getOpcode() == ISD::SRL) {
+    if (Srl.getOpcode() == ISD::SRL) {
       // cvt_f32_ubyte0 (srl x, 16) -> cvt_f32_ubyte2 x
       // cvt_f32_ubyte1 (srl x, 16) -> cvt_f32_ubyte3 x
       // cvt_f32_ubyte0 (srl x, 8) -> cvt_f32_ubyte1 x
 
-      if (const ConstantSDNode *C = dyn_cast<ConstantSDNode>(Src.getOperand(1))) {
+      if (const ConstantSDNode *C =
+              dyn_cast<ConstantSDNode>(Srl.getOperand(1))) {
+        Srl = DAG.getZExtOrTrunc(Srl.getOperand(0), SDLoc(Srl.getOperand(0)),
+                                 EVT(MVT::i32));
+
         unsigned SrcOffset = C->getZExtValue() + 8 * Offset;
         if (SrcOffset < 32 && SrcOffset % 8 == 0) {
           return DAG.getNode(AMDGPUISD::CVT_F32_UBYTE0 + SrcOffset / 8, DL,
-                             MVT::f32, Src.getOperand(0));
+                             MVT::f32, Srl);
         }
       }
     }
 
     APInt Demanded = APInt::getBitsSet(32, 8 * Offset, 8 * Offset + 8);
 
     APInt KnownZero, KnownOne;
     TargetLowering::TargetLoweringOpt TLO(DAG, !DCI.isBeforeLegalize(),
                                           !DCI.isBeforeLegalizeOps());
     const TargetLowering &TLI = DAG.getTargetLoweringInfo();
     if (TLO.ShrinkDemandedConstant(Src, Demanded) ||
         TLI.SimplifyDemandedBits(Src, Demanded, KnownZero, KnownOne, TLO)) {
       DCI.CommitTargetLoweringOpt(TLO);
     }
 
     break;
   }
-
+  case ISD::SINT_TO_FP:
   case ISD::UINT_TO_FP: {
     return performUCharToFloatCombine(N, DCI);
   }
   case ISD::FADD: {
     if (DCI.getDAGCombineLevel() < AfterLegalizeDAG)
       break;
 
     EVT VT = N->getValueType(0);