RE: [PATCH v4] PR middle-end/60281

[Bernd Edlinger <bernd dot edlinger at hotmail dot de>]

Hi Lin,

On Fri, 28 Feb 2014 19:14:11, lin zuojian wrote:
>
> ä 2014å02æ28æ 15:58, lin zuojian åé:
>> Hi Bernd,
>> I agree you with the mode problem.
>>
>> And I have not change the stack alignment.What I change is the virtual
>> register base's alignment.

I tried your patch on this test case:

gcc -O2 -fsanitize=address -S gcc/testsuite/c-c++-common/asan/pr59063-1.c

and compared the stack usage with and without patch:

without patch:

ÂÂÂÂÂÂÂ @ Function supports interworking.
ÂÂÂÂÂÂÂ @ args = 0, pretend = 0, frame = 96
ÂÂÂÂÂÂÂ @ frame_needed = 0, uses_anonymous_args = 0
ÂÂÂÂÂÂÂ stmfdÂÂ sp!, {r4, r5, r6, r7, r8, lr}
ÂÂÂÂÂÂÂ ldrÂÂÂÂ r3, .L12
ÂÂÂÂÂÂÂ ldrÂÂÂÂ r3, [r3]
ÂÂÂÂÂÂÂ subÂÂÂÂ sp, sp, #96
ÂÂÂÂÂÂÂ cmpÂÂÂÂ r3, #0
ÂÂÂÂÂÂÂ movÂÂÂÂ r5, sp
ÂÂÂÂÂÂÂ movÂÂÂÂ r8, sp

and with patch:

ÂÂÂÂÂÂÂ @ Function supports interworking.
ÂÂÂÂÂÂÂ @ args = 0, pretend = 0, frame = 128
ÂÂÂÂÂÂÂ @ frame_needed = 0, uses_anonymous_args = 0
ÂÂÂÂÂÂÂ stmfdÂÂ sp!, {r4, r5, r6, r7, r8, lr}
ÂÂÂÂÂÂÂ ldrÂÂÂÂ r3, .L12
ÂÂÂÂÂÂÂ ldrÂÂÂÂ r3, [r3]
ÂÂÂÂÂÂÂ subÂÂÂÂ sp, sp, #128
ÂÂÂÂÂÂÂ cmpÂÂÂÂ r3, #0
ÂÂÂÂÂÂÂ addÂÂÂÂ r3, sp, #128
ÂÂÂÂÂÂÂ bicÂÂÂÂ r4, r3, #31
ÂÂÂÂÂÂÂ subÂÂÂÂ r4, r4, #96
ÂÂÂÂÂÂÂ movÂÂÂÂ r8, r4

So, that is what I mean: this patch makes the stack grow by
32 bytes, just because the emit_stack_protection uses SImode,
with unaligned addresses which is not possible for ARM, and
not optimal for X86_64.

Why not use the true alignment value in set_mem_align?
And simply let get_best_mode() choose the right mode to
use for that alignment.

IMHO it should not be necessary to use STRICT_ALIGNMENT
everywhere, because get_best_mode should know what mode
is appropriate for which alignment value.


Regards
Bernd.

>> Realignment must be make in !STRICT_ALIGNMENT machine,or emitting the
>> efficient code is impossible.
> Sorry, it should be "Realignment must be make in STRICT_ALIGNMENT machine".
>> For example 4 set mem:QI X,REG:QI Y will not combine into one set mem:SI
>> X1,REG:SI Y1,if X is not mentioned as SI mode aligned.
>> To make sure X is SI mode algined,virtual register base must be realigned.
>>
>> For this patch,I only intent to make it right.Making it best is next task.
>> --
>> Regards
>> lin zuojian.
>>
>> ä 2014å02æ28æ 15:47, Bernd Edlinger åé:
>>> Hi,
>>>
>>> I see the problem too.
>>>
>>> But I think it is not necessary to change the stack alignment
>>> to solve the problem.
>>>
>>> It appears to me that the code in asan_emit_stack_protection
>>> is just wrong. It uses SImode when the memory is not aligned
>>> enough for that mode. This would not happen if that code
>>> is rewritten to use get_best_mode, and by the way, even on
>>> x86_64 the emitted code is not optimal, because that target
>>> could work with DImode more efficiently.
>>>
>>> So, to fix that, it would be better to concentrate on that function,
>>> and use word_mode instead of SImode, and let get_best_mode
>>> choose the required mode.
>>>
>>>
>>> Regards
>>> Bernd Edlinger.
>