This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [m68k 10/13] cleanup fp compare
Roman Zippel <zippel@linux-m68k.org> writes:
> On Mon, 5 Feb 2007, Richard Sandiford wrote:
>> > What difference does it make at this point?
>>
>> The difference is that conditions are AIUI canonicalised at the tree level
>> in such a way that the second operand is likely to the non-constant once.
>
> Ah, I forgot about this.
Oops, sorry, I typoed. I meant: ...in such a way that the second operand
is likely to the _constant one_. (I think you knew that, but just for
the record.)
> Ok, below is an updated patch, which also rejects all constants for
> ColdFire. (Ligthly tested.)
Thanks. It looks good to me, except for one thing:
> +;; Special case of general_src_operand, which rejects a few fp
> +;; constants (which we prefer in registers) before reload.
> +
> +(define_predicate "fp_src_operand"
> + (match_operand 0 "general_src_operand")
> +{
> + return !CONSTANT_P (op) || (!TARGET_COLDFIRE_FPU
> + && !standard_68881_constant_p (op))
> + || reload_in_progress || reload_completed;
> +})
...the ColdFire patterns shouldn't really accept constants at any
stage. The different pre-reload and mid/post-reload handling of
special constants doesn't apply to ColdFire either.
Also -- very minor -- TARGET_68881 would be better than
!TARGET_COLDFIRE_FPU, especially as you're calling a function
with 68881 in its name anyway.
So I think the predicate should look something like this:
-------------------------------------------------------------------------
(define_predicate "fp_src_operand"
(match_operand 0 "general_src_operand")
{
return (!CONSTANT_P (op)
|| (TARGET_68881
&& (!standard_68881_constant_p (op)
|| reload_in_progress
|| reload_completed)));
})
-------------------------------------------------------------------------
The patch looks good to me otherwise FWIW.
Richard