[MIPS] Fix illegal type assert in single-float mode

[ZirconiumX]

Summary

An fp_to_sint node would be incorrectly lowered to a TruncIntFP node in single-float mode. This would trigger an "Unexpected illegal type!" assert.

Many thanks to Tim Northover for helping me through this.

[atanasyan]

test/CodeGen/Mips/f32-to-i64-single-float.ll

1 ↗

(On Diff #164493)

It looks like you miss any CHECK statements. I understand that without the fix this test case triggers assert, but it's better to express expected assembler code and check them.

[ZirconiumX]

test/CodeGen/Mips/f32-to-i64-single-float.ll

1 ↗

(On Diff #164493)

The expected assembly code is...complicated, and while I've been dipping my toes into MIPS because of the PS2, even on -O0, LLVM inlines the conversion rather than producing something convenient to check like a library call. I'm not sure what exactly to check for, because my MIPS assembly knowledge isn't perfect.

[sdardis]

test/CodeGen/Mips/f32-to-i64-single-float.ll

1 ↗

(On Diff #164493)

You can use update_llc_checks.py to generate the expected / current assembly result rather than hand-crafting CHECK lines. You'll need to change the -march=mips64el to -mtriple=mips64el-mti-linux-gnu .

3 ↗

(On Diff #164493)

Also, remove the '#0' here and a add a comment describing the test case (a line or two is fine).

[ZirconiumX]

I've ran update_llc_checks.py on my testcase, and added a brief description of the test as requested.

[ZirconiumX]

These should be fixed in the new diff.

[atanasyan]

LGTM. Thanks. Do you have commit access?

[ZirconiumX]

As this is my first patch, I do not, no.

llvm/trunk/lib/Target/Mips/MipsISelLowering.cpp

static SDValue lowerUnalignedIntStore(StoreSDNode *SD, SelectionDAG &DAG,
   // to
   //  (sdl val, (add baseptr, 7))
   //  (sdr val, baseptr)
   SDValue SDL = createStoreLR(MipsISD::SDL, DAG, SD, Chain, IsLittle ? 7 : 0);
   return createStoreLR(MipsISD::SDR, DAG, SD, SDL, IsLittle ? 0 : 7);
 }
 
 // Lower (store (fp_to_sint $fp) $ptr) to (store (TruncIntFP $fp), $ptr).
-static SDValue lowerFP_TO_SINT_STORE(StoreSDNode *SD, SelectionDAG &DAG) {
+static SDValue lowerFP_TO_SINT_STORE(StoreSDNode *SD, SelectionDAG &DAG,
+                                     bool SingleFloat) {
   SDValue Val = SD->getValue();
 
-  if (Val.getOpcode() != ISD::FP_TO_SINT)
+  if (Val.getOpcode() != ISD::FP_TO_SINT ||
+      (Val.getValueSizeInBits() > 32 && SingleFloat))
     return SDValue();
 
   EVT FPTy = EVT::getFloatingPointVT(Val.getValueSizeInBits());
   SDValue Tr = DAG.getNode(MipsISD::TruncIntFP, SDLoc(Val), FPTy,
                            Val.getOperand(0));
   return DAG.getStore(SD->getChain(), SDLoc(SD), Tr, SD->getBasePtr(),
                       SD->getPointerInfo(), SD->getAlignment(),
                       SD->getMemOperand()->getFlags());
 }
 
 SDValue MipsTargetLowering::lowerSTORE(SDValue Op, SelectionDAG &DAG) const {
   StoreSDNode *SD = cast<StoreSDNode>(Op);
   EVT MemVT = SD->getMemoryVT();
 
   // Lower unaligned integer stores.
   if (!Subtarget.systemSupportsUnalignedAccess() &&
       (SD->getAlignment() < MemVT.getSizeInBits() / 8) &&
       ((MemVT == MVT::i32) || (MemVT == MVT::i64)))
     return lowerUnalignedIntStore(SD, DAG, Subtarget.isLittle());
 
-  return lowerFP_TO_SINT_STORE(SD, DAG);
+  return lowerFP_TO_SINT_STORE(SD, DAG, Subtarget.isSingleFloat());
 }
 
 SDValue MipsTargetLowering::lowerEH_DWARF_CFA(SDValue Op,
                                               SelectionDAG &DAG) const {
 
   // Return a fixed StackObject with offset 0 which points to the old stack
   // pointer.
   MachineFrameInfo &MFI = DAG.getMachineFunction().getFrameInfo();
   EVT ValTy = Op->getValueType(0);
   int FI = MFI.CreateFixedObject(Op.getValueSizeInBits() / 8, 0, false);
   return DAG.getFrameIndex(FI, ValTy);
 }
 
 SDValue MipsTargetLowering::lowerFP_TO_SINT(SDValue Op,
                                             SelectionDAG &DAG) const {
+  if (Op.getValueSizeInBits() > 32 && Subtarget.isSingleFloat())
+    return SDValue();
+
   EVT FPTy = EVT::getFloatingPointVT(Op.getValueSizeInBits());
   SDValue Trunc = DAG.getNode(MipsISD::TruncIntFP, SDLoc(Op), FPTy,
                               Op.getOperand(0));
   return DAG.getNode(ISD::BITCAST, SDLoc(Op), Op.getValueType(), Trunc);
 }
 
 //===----------------------------------------------------------------------===//
 //                      Calling Convention Implementation

llvm/trunk/test/CodeGen/Mips/f32-to-i64-single-float.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=mips64el-mti-linux-gnu -mattr=+single-float < %s | FileCheck %s
+; This test casts a 32-bit float to a 64-bit int. This would cause a crash due
+; to LLVM incorrectly lowering the float on single-float platforms.
+
+define void @foo(float* %in, i64* %out) {
+; CHECK-LABEL: foo:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    daddiu $sp, $sp, -16
+; CHECK-NEXT:    .cfi_def_cfa_offset 16
+; CHECK-NEXT:    sd $5, 0($sp)
+; CHECK-NEXT:    sd $4, 8($sp)
+; CHECK-NEXT:    lwc1 $f0, 0($4)
+; CHECK-NEXT:    mfc1 $1, $f0
+; CHECK-NEXT:    lui $2, 32640
+; CHECK-NEXT:    and $2, $1, $2
+; CHECK-NEXT:    srl $2, $2, 23
+; CHECK-NEXT:    lui $3, 127
+; CHECK-NEXT:    ori $3, $3, 65535
+; CHECK-NEXT:    addiu $4, $zero, 150
+; CHECK-NEXT:    subu $4, $4, $2
+; CHECK-NEXT:    and $3, $1, $3
+; CHECK-NEXT:    lui $6, 128
+; CHECK-NEXT:    or $3, $3, $6
+; CHECK-NEXT:    dsll $3, $3, 32
+; CHECK-NEXT:    dsrl $3, $3, 32
+; CHECK-NEXT:    dsrlv $4, $3, $4
+; CHECK-NEXT:    addiu $6, $2, -150
+; CHECK-NEXT:    dsllv $3, $3, $6
+; CHECK-NEXT:    addiu $2, $2, -127
+; CHECK-NEXT:    slti $6, $2, 24
+; CHECK-NEXT:    movz $4, $3, $6
+; CHECK-NEXT:    sra $1, $1, 31
+; CHECK-NEXT:    xor $3, $4, $1
+; CHECK-NEXT:    dsubu $1, $3, $1
+; CHECK-NEXT:    slti $2, $2, 0
+; CHECK-NEXT:    movn $1, $zero, $2
+; CHECK-NEXT:    sd $1, 0($5)
+; CHECK-NEXT:    jr $ra
+; CHECK-NEXT:    daddiu $sp, $sp, 16
+entry:
+  %in.addr = alloca float*, align 8
+  %out.addr = alloca i64*, align 8
+  store float* %in, float** %in.addr, align 8
+  store i64* %out, i64** %out.addr, align 8
+  %0 = load float*, float** %in.addr, align 8
+  %1 = load float, float* %0, align 4
+  %conv = fptosi float %1 to i64
+  %2 = load i64*, i64** %out.addr, align 8
+  store i64 %conv, i64* %2, align 8
+  ret void
+}