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.