GlobalISel: Restrict narrow scalar for fptoui/fptosi results

[arsenm]

Summary

This practically only works for the f16 case AMDGPU uses, not wider
types.

Fixes bug 49710 by failing legalization.

[nikic]

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp

4721

Shouldn't this check the NarrowTy, rather than the old DstTy?

[arsenm]

Check new result type

[foad]

The code change looks fine to me. The added tests aren't actually testing any behaviour that has changed, are they? Is there any reasonable way to add tests for the unable-to-legalize case?

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp

4714

There's no need to introduce NarrowDstTy, just use NarrowTy throughout.

4719

Could invert the condition and early-out here but it doesn't really matter.

[arsenm]

Style changes

[nikic]

Reverse ping, I believe @foad's comment regarding the test is still open.

[arsenm]

In D99469#2657766, @foad wrote:

The code change looks fine to me. The added tests aren't actually testing any behaviour that has changed, are they? Is there any reasonable way to add tests for the unable-to-legalize case?

That is what's tested here. The no-ops in the new tests demonstrate they weren't legalized.

[nikic]

LGTM

(Just to be sure, I checked that the added tests did fail previously.)

[arsenm]

83a25a101051b404bec1a5ba9cb867705f31262d

llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h

public:
 
   LegalizeResult narrowScalarShiftByConstant(MachineInstr &MI, const APInt &Amt,
                                              LLT HalfTy, LLT ShiftAmtTy);
 
   LegalizeResult narrowScalarShift(MachineInstr &MI, unsigned TypeIdx, LLT Ty);
   LegalizeResult narrowScalarAddSub(MachineInstr &MI, unsigned TypeIdx,
                                     LLT NarrowTy);
   LegalizeResult narrowScalarMul(MachineInstr &MI, LLT Ty);
+  LegalizeResult narrowScalarFPTOI(MachineInstr &MI, unsigned TypeIdx, LLT Ty);
   LegalizeResult narrowScalarExtract(MachineInstr &MI, unsigned TypeIdx, LLT Ty);
   LegalizeResult narrowScalarInsert(MachineInstr &MI, unsigned TypeIdx, LLT Ty);
 
   LegalizeResult narrowScalarBasic(MachineInstr &MI, unsigned TypeIdx, LLT Ty);
   LegalizeResult narrowScalarExt(MachineInstr &MI, unsigned TypeIdx, LLT Ty);
   LegalizeResult narrowScalarSelect(MachineInstr &MI, unsigned TypeIdx, LLT Ty);
   LegalizeResult narrowScalarCTLZ(MachineInstr &MI, unsigned TypeIdx, LLT Ty);
   LegalizeResult narrowScalarCTTZ(MachineInstr &MI, unsigned TypeIdx, LLT Ty);

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp

LegalizerHelper::LegalizeResult LegalizerHelper::narrowScalar(MachineInstr &MI,
   case TargetOpcode::G_PTRMASK: {
     if (TypeIdx != 1)
       return UnableToLegalize;
     Observer.changingInstr(MI);
     narrowScalarSrc(MI, NarrowTy, 2);
     Observer.changedInstr(MI);
     return Legalized;
   }
-  case TargetOpcode::G_FPTOUI: {
-    if (TypeIdx != 0)
-      return UnableToLegalize;
-    Observer.changingInstr(MI);
-    narrowScalarDst(MI, NarrowTy, 0, TargetOpcode::G_ZEXT);
-    Observer.changedInstr(MI);
-    return Legalized;
-  }
-  case TargetOpcode::G_FPTOSI: {
-    if (TypeIdx != 0)
-      return UnableToLegalize;
-    Observer.changingInstr(MI);
-    narrowScalarDst(MI, NarrowTy, 0, TargetOpcode::G_SEXT);
-    Observer.changedInstr(MI);
-    return Legalized;
-  }
+  case TargetOpcode::G_FPTOUI:
+  case TargetOpcode::G_FPTOSI:
+    return narrowScalarFPTOI(MI, TypeIdx, NarrowTy);
   case TargetOpcode::G_FPEXT:
     if (TypeIdx != 0)
       return UnableToLegalize;
     Observer.changingInstr(MI);
     narrowScalarDst(MI, NarrowTy, 0, TargetOpcode::G_FPEXT);
     Observer.changedInstr(MI);
     return Legalized;
   }
LegalizerHelper::narrowScalarMul(MachineInstr &MI, LLT NarrowTy) {
   ArrayRef<Register> DstRegs(
       IsMulHigh ? &DstTmpRegs[DstTmpParts / 2] : &DstTmpRegs[0], NumDstParts);
   MIRBuilder.buildMerge(DstReg, DstRegs);
   MI.eraseFromParent();
   return Legalized;
 }
 
 LegalizerHelper::LegalizeResult
+LegalizerHelper::narrowScalarFPTOI(MachineInstr &MI, unsigned TypeIdx,
+                                   LLT NarrowTy) {
+  if (TypeIdx != 0)
+    return UnableToLegalize;
+
+  bool IsSigned = MI.getOpcode() == TargetOpcode::G_FPTOSI;
+
+  Register Src = MI.getOperand(1).getReg();
+  LLT SrcTy = MRI.getType(Src);
+
+  // If all finite floats fit into the narrowed integer type, we can just swap
+  // out the result type. This is practically only useful for conversions from
+  // half to at least 16-bits, so just handle the one case.
+  if (SrcTy.getScalarType() != LLT::scalar(16) ||
+      NarrowTy.getScalarSizeInBits() < (IsSigned ? 17 : 16))
+    return UnableToLegalize;
+
+  Observer.changingInstr(MI);
+  narrowScalarDst(MI, NarrowTy, 0,
+                  IsSigned ? TargetOpcode::G_SEXT : TargetOpcode::G_ZEXT);
+  Observer.changedInstr(MI);
+  return Legalized;
+}
+
+LegalizerHelper::LegalizeResult
 LegalizerHelper::narrowScalarExtract(MachineInstr &MI, unsigned TypeIdx,
                                      LLT NarrowTy) {
   if (TypeIdx != 1)
     return UnableToLegalize;
 
   uint64_t NarrowSize = NarrowTy.getSizeInBits();
 
   int64_t SizeOp1 = MRI.getType(MI.getOperand(1).getReg()).getSizeInBits();

llvm/test/CodeGen/AArch64/GlobalISel/legalize-fptoi.mir

bb.0:
     ; CHECK-LABEL: name: test_fptosi_v4s32
     ; CHECK: [[COPY:%[0-9]+]]:_(<4 x s32>) = COPY $q0
     ; CHECK: [[FPTOSI:%[0-9]+]]:_(<4 x s32>) = G_FPTOSI [[COPY]](<4 x s32>)
     ; CHECK: $q0 = COPY [[FPTOSI]](<4 x s32>)
     %0:_(<4 x s32>) = COPY $q0
     %1:_(<4 x s32>) = G_FPTOSI %0
     $q0 = COPY %1
 ...
+
+---
+name:            test_fptoui_s128_s32
+body: |
+  bb.0:
+    liveins: $w0
+    ; CHECK-LABEL: name: test_fptoui_s128_s32
+    ; CHECK: [[COPY:%[0-9]+]]:_(s32) = COPY $w0
+    ; CHECK: [[FPTOUI:%[0-9]+]]:_(s128) = G_FPTOUI [[COPY]](s32)
+    ; CHECK: $q0 = COPY [[FPTOUI]](s128)
+    %0:_(s32) = COPY $w0
+    %1:_(s128) = G_FPTOUI %0
+    $q0 = COPY %1
+...
+
+---
+name:            test_fptosi_s128_s32
+body: |
+  bb.0:
+    liveins: $w0
+    ; CHECK-LABEL: name: test_fptosi_s128_s32
+    ; CHECK: [[COPY:%[0-9]+]]:_(s32) = COPY $w0
+    ; CHECK: [[FPTOSI:%[0-9]+]]:_(s128) = G_FPTOSI [[COPY]](s32)
+    ; CHECK: $q0 = COPY [[FPTOSI]](s128)
+    %0:_(s32) = COPY $w0
+    %1:_(s128) = G_FPTOSI %0
+    $q0 = COPY %1
+...