This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] MIPS: 64bit floating point support for MIPS32R2


David Ung <davidu@mips.com> writes:
> Richard Sandiford wrote:
>>>+(define_insn "*movdf_hardfloat_mips32r2_fp64"
>>>+  [(set (match_operand:DF 0 "nonimmediate_operand" "=f,f,f,m,m,*f,*d,*d,*d,*m")
>>>+	(match_operand:DF 1 "move_operand" "f,G,m,f,G,*d,*f,*d*G,*m,*d"))]
>>>+  "TARGET_HARD_FLOAT && TARGET_DOUBLE_FLOAT && TARGET_FLOAT64
>> 
>> 
>> Missing !TARGET_64BIT here.  Might as well drop TARGET_DOUBLE_FLOAT
>> for consistency with your other patterns; it will never be false if
>> TARGET_FLOAT64.  As above, I think *movvdf_hardfloat_gp32_fp64
>> would be a better name.  
>
> hmm, that makes movdf_hardfloat_32bit needing !TARGET_64BIT aswell as 
> !TARGET_FLOAT64.

Either should be fine, but...

>>>+   && (register_operand (operands[0], DFmode)
>>>+       || reg_or_0_operand (operands[1], DFmode))"
>>>+  { return mips_output_move (operands[0], operands[1]); }
>>>+  [(set_attr "type"	"fmove,xfer,fpload,fpstore,store,xfer,xfer,arith,load,store")
>>>+   (set_attr "mode"	"DF")
>>>+   (set_attr "length"	"4,4,*,*,*,8,8,8,*,*")])
>> 
>> 
>> I think the second length should be 8, because it splits into an
>> mtc1 and an mthc1, but correct me if I'm wrong.  Looks good otherwise.
>
> aye.  For which case that would make the above pattern redendent.
> Could just use movdf_hardfloat_32bit.

...hmm, good spot.  I hadn't realised the two patterns were so regular.
So, let's do that then ;)  It would probably be worth adding a comment like:

;; This pattern applies to both !TARGET_FLOAT64 and TARGET_FLOAT64.

just to confirm that it has dual use.

>> The main missing thing I can see is the documentation changes
>> I mentioned before, including the effect that -mfp64 has on the ABI.
>> Otherwise, with the above fixed, and with the regressions from -mfp32
>> smoothed out, the patch looks good to go.
>
> ok, I'll add something in the "Generate code for the given ABI"
> section.  I am not sure if would be suitable to add a note under
> -mfp64, but I guess it does no harm.

Sounds good.  If you add something to the -mabi documentation,
I agree you don't need anything special under -mfp64.

I'm still a little worried about that cannot_change_mode_class hunk;
do you know off-hand why it's needed?

mips_class_max_nregs might also need changing btw.

Richard


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]