Jakub Jelinek <ja...@redhat.com> writes:
> On Tue, Oct 29, 2013 at 12:25:33PM +0000, Richard Sandiford wrote:
>> >> Any updates on this one? Note that this bug is a huge blocker for
>> >> using AddressSanitizer on ARM platforms.
>> >
>> > Sorry for the delay, I finally found time to look at it.
>> > While your patch fixes the issue, I wonder if for the case where
>> > shadow_mem before the asan_clear_shadow call is already of the form
>> > (mem (reg)) it isn't better to just adjust next asan_clear_shadow
>> > base from the end of the cleared region rather than from the start of it,
>> > because the end will be already in the right pseudo, while with your
>> > approach it needs to be done in a different register and potentially
>> > increase register pressure. So here is (completely untested) alternate
>> > fix:
>>
>> Hmm, it just seems wrong to be assigning to registers returned by force_reg
>> and expand_binop though. Would this variation be OK?
>
> Why?
Well, that was the traditional rule:
The caller must not alter the value in the register we return,
since we mark it as a "constant" register. */
rtx
force_reg (enum machine_mode mode, rtx x)
{
but maybe it doesn't matter now, since the constant register flag has
gone and since we seem to use REG_EQUAL rather than REG_EQUIV...
> If it is a pseudo, it is certainly a pseudo that isn't used for
> anything else, as it is the result of (base >> 3) + constant, if it isn't a
> pseudo, then supposedly it is better not to just keep adding the offsets to
> a base and thus not to use the end address from asan_clear_shadow.
> Your patch would force using the end address even if shadow_base
> is initially say some_reg + 32, I think it is better to use shadow_mem
> some_reg + 72 next time instead of some_other_reg + 40.
But I thought the register pressure thing was to avoid having the
original base live across the loop. Doesn't that apply for reg + offset
as well as plain reg? If we have to force some_reg + 32 into a
temporary and then loop on it, using the new base should avoid keeping
both it and some_reg live at the same time. Plus smaller offsets are
often better on some targets.
Thanks,
Richard
