[PATCH] D58722: [MIPS] [microMIPS] Pattern match TruncIntFP

Thu Mar 7 11:44:32 PST 2019

sdardis accepted this revision.
sdardis marked an inline comment as done.
sdardis added a comment.
This revision is now accepted and ready to land.

One nit inline, but otherwise LGTM.

================
Comment at: test/CodeGen/Mips/llvm-ir/fptosi.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=mips-linux-gnu   -mcpu=mips32                          -asm-show-inst | FileCheck %s -check-prefixes=ALL,NOT-MM,M32,M32-NOTFP64,M32D
+; RUN: llc < %s -mtriple=mips-linux-gnu   -mcpu=mips32r2                        -asm-show-inst | FileCheck %s -check-prefixes=ALL,NOT-MM,M32,M32-NOTFP64,M32R2
----------------
mbrkusanin wrote:
> sdardis wrote:
> > 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.
> 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).
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.

================
Comment at: test/CodeGen/Mips/llvm-ir/fptosi.ll:30
+; RUN:   FileCheck %s -check-prefixes=MMR2-SF
+; RUN: llc < %s -mtriple=mips-linux-gnu -mcpu=mips32r6 -mattr=+micromips,fp64 -asm-show-inst |\
+; RUN:   FileCheck %s -check-prefixes=MMR6-FP64
----------------
The ',fp64' isn't required here as microMIPSR6 requires a 64bit FPU.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58722/new/

https://reviews.llvm.org/D58722