[MIPS] [microMIPS] Pattern match TruncIntFP

[mbrkusanin]

Summary

A pattern needed to match TruncIntFP was missing. This was causing multiple
tests from llvm test suite to fail during compilation for micromips.

[sdardis]

Two comments, and some more inlined. Check the formatting of the commit message
body, it should be 80 cols max. Otherwise the log messages can look strange due
to wrapping (I'm using git).

Second, after committing this, add more RUN: lines to this test to cover the
likes of MIPS32, MIPS32r2 with and without FP64, with soft-float, MIPS-III, MIPS64,
and MIPSR6 along with microMIPSR6 to document current behaviour.

Otherwise, LGTM with my comments addressed without the test additions for other
FPU configurations.

test/CodeGen/Mips/trunc-w-s-mm.ll

1

This file should be in test/CodeGen/Mips/llvm-ir/fptosi.ll - that test directory is for testing the matching of LLVM IR constructs through ISel to machine code.

2

-relocation-model=pic is not needed for this test.

4

Small nit: For the test/CodeGen/Mips/llvm-ir/ directory, the comment describing the purpose of the test should be along the lines of 'Test that fptosi can be matched for MIPS targets for various FPU configurations' .

18–20

Cut these three lines from the test and perform fptosi directly on the float parameter. The alloca, store, load IR constructs are uninteresting from a testing perspective.

[mbrkusanin]

Pattern matches for other configurations are added. Thanks for mentioning that. I've also added -asm-show-inst to RUN: lines that show which version of TRUNC_W will be chosen since that is what this patch is doing.

[sdardis]

Some minor nits inlined, but otherwise the patch is more or less there.

lib/Target/Mips/MicroMipsInstrFPU.td

433–435

This pattern is duplicated by the one above except for the predicates. Drop the 'FGR_32' in the pattern above and remove this pattern. For pure FGR32 operand using instructions, they shouldn't need to distinguish between the 32bit or 64bit FPU configurations.

test/CodeGen/Mips/llvm-ir/fptosi.ll

2 ↗

(On Diff #189508)

Break the line with '\' after the '|', and only include only one prefix with -check-prefix. Just make sure it's unique in the tests. Drop the excessive whitespace between the arguments.

13 ↗

(On Diff #189508)

You should also test microMIPS softfloat for completeness sake.

15 ↗

(On Diff #189508)

This test is unnecessary as microMIPS32R6 requires a full 64-bit fpu. See Introduction to the microMIPS32 Architecture v6.0, Chapter 7, binary compatibility, page 103.

[mbrkusanin]

test/CodeGen/Mips/llvm-ir/fptosi.ll

2 ↗

(On Diff #189508)

Can you just clarify what you meant by 'unique'. Unique per RUN: line or unique per generated output with utils/update_llc_test_checks.py?
Right now it is changed to latter (first two RUN:s have same prefix 'M32' since they generate the same output).

[sdardis]

One nit inline, but otherwise LGTM.

test/CodeGen/Mips/llvm-ir/fptosi.ll

30 ↗

(On Diff #189705)

The ',fp64' isn't required here as microMIPSR6 requires a 64bit FPU.

2 ↗

(On Diff #189508)

I would agree with the latter interpretation (unique per output) in this case. Sorry for the ambivalence. There can be cases where mips32 and mips32r2 for example generate different output. In this case, as they generate the same output it's fine to merge the the check prefixes. utils/update_llc_test_checks.py does complain when multiple RUN: lines have the same check prefixes but different output.

[mbrkusanin]

Addressed review comments.

lib/Target/Mips/MicroMipsInstrFPU.td

def : MipsPat<(f64 (fpextend FGR32Opnd:$src)),
               (CVT_D64_S_MM FGR32Opnd:$src)>, ISA_MICROMIPS, FGR_64;
 def : MipsPat<(f32 (fpround AFGR64Opnd:$src)),
               (CVT_S_D32_MM AFGR64Opnd:$src)>, ISA_MICROMIPS, FGR_32;
 def : MipsPat<(f64 (fpextend FGR32Opnd:$src)),
               (CVT_D32_S_MM FGR32Opnd:$src)>, ISA_MICROMIPS, FGR_32;
 def : MipsPat<(MipsTruncIntFP AFGR64Opnd:$src),
               (TRUNC_W_MM AFGR64Opnd:$src)>, ISA_MICROMIPS32_NOT_MIPS32R6,
               FGR_32;
+def : MipsPat<(MipsTruncIntFP FGR32Opnd:$src),
+              (TRUNC_W_S_MM FGR32Opnd:$src)>, ISA_MICROMIPS32_NOT_MIPS32R6,
+              FGR_32;
 
 // Selects
 defm : MovzPats0<GPR32, FGR32, MOVZ_I_S_MM, SLT_MM, SLTu_MM, SLTi_MM, SLTiu_MM>,
        ISA_MICROMIPS32_NOT_MIPS32R6;
 defm : MovzPats1<GPR32, FGR32, MOVZ_I_S_MM, XOR_MM>,
        ISA_MICROMIPS32_NOT_MIPS32R6;
 
 defm : MovnPats<GPR32, FGR32, MOVN_I_S_MM, XOR_MM>,
        ISA_MICROMIPS32_NOT_MIPS32R6;
 
 defm : MovzPats0<GPR32, AFGR64, MOVZ_I_D32_MM, SLT_MM, SLTu_MM, SLTi_MM,
                  SLTiu_MM>,
        ISA_MICROMIPS32_NOT_MIPS32R6, FGR_32;
 defm : MovzPats1<GPR32, AFGR64, MOVZ_I_D32_MM, XOR_MM>,
        ISA_MICROMIPS32_NOT_MIPS32R6, FGR_32;
 defm : MovnPats<GPR32, AFGR64, MOVN_I_D32_MM, XOR_MM>,
        ISA_MICROMIPS32_NOT_MIPS32R6, FGR_32;

test/CodeGen/Mips/trunc-w-s-mm.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=mipsel-linux-gnu -mattr=+micromips -relocation-model=pic < %s | FileCheck %s
+
+; Test that should fail if there is no DAGISel pattern for TruncIntFP to
+; to TRUNC_W_S_MM
+
+define i32 @test(float %t) {
+; CHECK-LABEL: test:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    addiu $sp, $sp, -8
+; CHECK-NEXT:    .cfi_def_cfa_offset 8
+; CHECK-NEXT:    swc1 $f12, 4($sp)
+; CHECK-NEXT:    trunc.w.s $f0, $f12
+; CHECK-NEXT:    mfc1 $2, $f0
+; CHECK-NEXT:    jr $ra
+; CHECK-NEXT:    addiu $sp, $sp, 8
+entry:
+  %t.addr = alloca float, align 4
+  store float %t, float* %t.addr, align 4
+  %0 = load float, float* %t.addr, align 4
+  %conv = fptosi float %0 to i32
+  ret i32 %conv
+}