Yvan Roux <yvan.r...@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
