Re: [PATCH][explow] PR target/85173: validize memory before passing it on to target probe_stack

[Uros Bizjak <ubizjak at gmail dot com>]

On Tue, Apr 10, 2018 at 12:26 AM, Jeff Law <law@redhat.com> wrote:
> On 04/05/2018 08:20 AM, Kyrill Tkachov wrote:
>> Hi all,
>>
>> In this PR the expansion code emits an invalid memory address for the
>> stack probe, which the backend fails to recognise.
>> The address is created explicitly in
>> anti_adjust_stack_and_probe_stack_clash in explow.c and passed down to
>> gen_probe_stack
>> without any validation in emit_stack_probe.
>>
>> This patch fixes the ICE by calling validize_mem on the memory location
>> before passing it down to the target.
>> Jakub pointed out that we also want to create valid addresses for the
>> probe_stack_address case, so this patch
>> creates an expand operand and legitimizes it before passing it down to
>> the probe_stack_address expander.
>>
>> This patch passes bootstrap and testing on arm-none-linux-gnueabihf and
>> aarch64-none-linux-gnu
>> and ppc64le-redhat-linux on gcc112 in the compile farm.
>>
>> Is this ok for trunk?
>>
>> Thanks,
>> Kyrill
>>
>> P.S. Uros, the alpha probe_stack expander in alpha.md seems incompatible
>> with the way the probe_stack name is
>> used in the midend. It accepts a const_int operand that is used as an
>> offset from the stack pointer, rather than accepting
>> a full memory operand like other targets. Do you think it's better to
>> rename the probe_stack pattern there to something
>> that doesn't conflict with the name the midend assumes?
>>
>> 2018-04-05  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>
>>     PR target/85173
>>     * explow.c (emit_stack_probe): Call validize_mem on memory location
>>     before passing it to gen_probe_stack.  Create address operand and
>>     legitimize it for the probe_stack_address case.
>>
>> 2018-04-05  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>
>>     PR target/85173
>>     * gcc.target/arm/pr85173.c: New test.
> Alpha should be fixed -- the docs clearly state that the operand is "the
> memory reference in the stack that needs to be probed".  Just passing in
> the offset seems wrong.

This pattern has to be renamed to not clash with the standard pattern name.

I'm testing the attached patch.

Uros.

Attachment: a.diff.txt
Description: Text document