This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: RFC: patch to build GCC for arm with LRA
- From: Richard Sandiford <rdsandiford at googlemail dot com>
- To: Yvan Roux <yvan dot roux at linaro dot org>
- Cc: Vladimir Makarov <vmakarov at redhat dot com>, Richard Earnshaw <rearnsha at arm dot com>, gcc-patches <gcc-patches at gcc dot gnu dot org>, Marcus Shawcroft <marcus dot shawcroft at arm dot com>, Ramana Radhakrishnan <Ramana dot Radhakrishnan at arm dot com>, Matthew Gretton-Dann <matthew dot gretton-dann at linaro dot org>, Richard Henderson <rth at redhat dot com>
- Date: Wed, 11 Sep 2013 18:49:08 +0100
- Subject: Re: RFC: patch to build GCC for arm with LRA
- Authentication-results: sourceware.org; auth=none
- References: <CAD57uCfSsANZvkMHmKphMHRnAxJNXGBxGEmSfhHSYwR7Jg9H-Q at mail dot gmail dot com> <877geri40k dot fsf at talisman dot default> <522D06E4 dot 1050602 at redhat dot com> <CAD57uCfpzd-7fKODP1oyWhY9f1QW2J=3BpMT3LAUMqC82Xvy0g at mail dot gmail dot com> <87zjrmgytu dot fsf at talisman dot default> <87vc2agymo dot fsf at talisman dot default> <CAD57uCfU3LMX+JM3yaJopqsp_rkJ3oyJv5g5VmJ5-9iyQvEMvQ at mail dot gmail dot com> <87wqmpg5n4 dot fsf at talisman dot default> <CAD57uCe-GrDqMHTNWtFcLRNDDVVr0_ZUg5E+zwaJdkCquNHagQ at mail dot gmail dot com> <87txhsmql0 dot fsf at talisman dot default> <CAD57uCchQSHKvaaA_Uq4rroiV9sopPgZgfTXnBJM-YpmWaHwdA at mail dot gmail dot com> <87li33n78h dot fsf at talisman dot default> <CAD57uCfsX4o39hPsgMBJ6K6E7p4YFrdqVFQdKAQHPHBADYAwnA at mail dot gmail dot com>
Yvan Roux <yvan.roux@linaro.org> writes:
> Yes indeed ! here is a fixed patch.
>
> In strip_address_mutations we now have 3 if/else if statements with
> the same body which could be factorized in:
>
> if ((GET_RTX_CLASS (code) == RTX_UNARY)
> /* Things like SIGN_EXTEND, ZERO_EXTEND and TRUNCATE can be
> used to convert between pointer sizes. */
> || (lsb_bitfield_op_p (*loc))
> /* Bitfield operations [SIGN|ZERO]_EXTRACT from the least significant
> bit can be used too. */
> || (code == AND && CONST_INT_P (XEXP (*loc, 1))))
> /* (and ... (const_int -X)) is used to align to X bytes. */
> loc = &XEXP (*loc, 0);
>
> if you think that it doesn't affect too much the readability.
Yeah, good point. TBH I prefer it with separate ifs though, because the
three cases are dealing with three different types of rtl (unary, binary
and ternary). But I don't mind much either way.
The new patch looks good to me, thanks. Just one minor style nit:
"return false" rather than "return 0" for the bool. Maybe also change:
/* Bitfield operations [SIGN|ZERO]_EXTRACT from the least significant
bit can be used too. */
to something like:
/* A [SIGN|ZERO]_EXTRACT from the least significant bit effectively
acts as a combined truncation and extension. */
I really will try to make that my last comment and leave things open
for an official review :-)
Thanks,
Richard