Hi,
On Tue, 27 May 2014 11:06:21, Richard Biener wrote:
>
> But the coment previously read
>
> /* A constant address in TO_RTX can have VOIDmode, we must not try
> to call force_reg for that case. Avoid that case. */
>
> and you are removing that check. So I guess you want to retain
>
> && GET_MODE (XEXP (to_rtx, 0)) != VOIDmode
>
well Richard, this code is definitely out-dated, with no test cases,
no good explanation at all. Obviously force_reg is no longer used
in that code path since years.
Actually I'd really like to clean up this code...
Furthermore nobody knows how to satisfy the condition
"MEM_P (to_rtx) && GET_MODE(XEXP(to_rtx,0) == VOIDmode".
A MEM_REF, referring to a SYMBOL_REF always seems to have a REG
in XEXP(to_rtx,0) which always is of type Pmode. (Which may be
specific to the ARM target).
And even if the MEM_REF would directly have a pointer to a
SYMBOL_REF as argument 0, it would also have pointer mode.
I looked for gen_rtx_SYMBOL_REF, and they all seem to use Pmode.
My feeling is that it is impossible to write an example where this
condition is false, and it could even be something like a
gcc_assert(GET_MODE (XEXP (to_rtx, 0)) != VOIDmode);
At least this condition seems not to trigger on normal SYMBOL_REFs.
If you really don't like to touch that part, please let me know.
I could let it, as it is, however that would not be my preference.
> or investigate why we don't need to avoid this case anymore. In fact,
> that's probably the only change necessary, just drop the == BLKmode
> check? Your lengthy description doesn't reveal if you investigated that.
> It seems to me it tries to avoid ajdusting MEM (symbol-ref) for example?
> Maybe also for optimization reasons?
>
The example code can be modified to test the conditions.
For instance to understand, why the condition
"MEM_ALIGN (to_rtx) == GET_MODE_ALIGNMENT (mode1)"
is too strict, while
"MEM_ALIGN (to_rtx)>= GET_MODE_ALIGNMENT (mode1) "
generates better code, simply add an alignment attribute
to the variable A a[6] __attribute__((aligned(32)));
This did not reach the if block at all, and the generated code, was
using mov instructions, even before my patch. But in this example,
we can see, there is no reason to restrict this only to exactly aligned
values.
The proposed condition on this if statement has some similarity
with the logic in store_field(). If the field is aligned in a certain way,
the access will likely be a single move instruction, and it is superior
code, to have only one constant offset part folded together. This would
naturally be different for bit-field accesses, that's what why the rest
of the conditions should not change.
> Adding a new comment before the block reflecting your analysis above
> would be nice as well.
>
Yes, I will add a comment, and post and updated version of the patch tomorrow.
Thanks
Bernd.
