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

Sasa Stankovic Sasa.Stankovic at imgtec.com
Sun Jul 13 03:22:13 PDT 2014


> 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.

OK.

================
Comment at: lib/Target/Mips/MipsMachineFunction.h:144
@@ +143,3 @@
+  // O32 FPXX ABI is enabled.
+  int BuildPairF64_FI;
+
----------------
Daniel Sanders wrote:
> 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 '///'.
Done.

================
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
+
----------------
Daniel Sanders wrote:
> 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.
I added a new test for dtmc1. Existing tests when compiled for n64 receive double arguments in float registers, so dmtc1 is not used.

I also added RUN-TODO lines.

http://reviews.llvm.org/D4465






More information about the llvm-commits mailing list