[PATCH] Expand BuildPairF64 by spilling to stack slot and reloading from it

Daniel Sanders daniel.sanders at imgtec.com
Sat Jul 12 04:14:04 PDT 2014


>>! In D4465#13, @sstankovic wrote:
>> It just occurred to me that we haven't covered moves from the FPU. They need to do the same thing.
> 
> Should this be part of this patch or a separate patch?

It can be a separate patch. I have an unfinished implementation of it for FP64A in my working copy at the moment. The code will be very similar between FP64A and FPXX and has very similar test cases so I could do both in the same patch if you like.

================
Comment at: lib/Target/Mips/MipsMachineFunction.h:144
@@ +143,3 @@
+  // O32 FPXX ABI is enabled.
+  int BuildPairF64_FI;
+
----------------
I see you are using -1 to mean invalid. -1 is normally a valid FrameIndex (negative numbers are used for stack objects with a fixed offset) so we should mention that -1 is used as an invalid value.

Also, please turn this comment into a doxygen comment using '///'.

================
Comment at: test/CodeGen/Mips/fpxx.ll:1-2
@@ +1,3 @@
+; RUN: llc -march=mipsel -mcpu=mips32 < %s | FileCheck %s -check-prefix=NO-FPXX
+; RUN: llc -march=mipsel -mcpu=mips32 -mattr=fpxx < %s | FileCheck %s -check-prefix=FPXX
+
----------------
Sasa Stankovic wrote:
> Daniel Sanders wrote:
> > It would be good to test the mtc1/mthc1 (mips32r2) and dmtc1 (mips4 or mips64r1) cases in this test. We cover them elsewhere as a side-effect of testing something else but it would be good to cover them deliberately in this test.
> I added the test for mips32r2. What should the mips64r1/mips4 case test?
They should test that the NOFPXX variant produces a dmtc1 and that the FPXX variant reports an error saying that FPXX is not permitted with the N32/N64 ABI's.

It's not strictly necessary, but it would be good to leave a RUN-TODO: line for the O32 (-mattr=-n64,+o32) on MIPS4/MIPS64r1 with and without FPXX cases. We don't support O32 on 64-bit ISA's at the moment but it will be helpful to be able to find all the tests easily (with something like 'grep -r RUN-TODO . | grep o32') when we come to support it.

http://reviews.llvm.org/D4465






More information about the llvm-commits mailing list