Re: [PATCH] [RFC] PR target/52813 and target/11807

[Bernd Edlinger <bernd.edlinger@hotmail.de>]

On 12/17/18 2:35 PM, Richard Sandiford wrote:
> Christophe Lyon <christophe.lyon@linaro.org> writes:
>> On Mon, 17 Dec 2018 at 12:47, Richard Sandiford
>> <richard.sandiford@arm.com> wrote:
>>>
>>> Dimitar Dimitrov <dimitar@dinux.eu> writes:
>>>> On Sun, Dec 16 2018 at 14:36:26 EET Bernd Edlinger wrote:
>>>>> Hi,
>>>>>
>>>>> if I understood that right, then clobbering sp is and has always been
>>>>> ignored.
>>>
>>> PR77904 was about the clobber not being ignored, so the behaviour
>>> hasn't been consistent.
>>>
>>> I'm also not sure it was always ignored in recent sources.  The clobber
>>> does get added to the associated rtl insn, and it'd be surprising if
>>> that never had an effect.
>>>
>>>>> If that is right, then I would much prefer a warning, that says exactly
>>>>> that, because that would also help to understand why removing that clobber
>>>>> statement is safe even for old gcc versions.
>>>
>>> If the asm does leave sp with a different value, then it's never been safe,
>>> regardless of the gcc version.  That's why an error seems more appropriate.
>>>
>>>> Thank you. Looks like general consensus is to have a warning. See attached
>>>> patch that switches the error to a warning.
>>>
>>> I don't think there's a good reason to treat this differently from the
>>> preexisting PIC register error.  If the argument for making it a warning
>>> rather than an error is that the asm might happen to work by accident,
>>> then the same is true for the PIC register.
>>>
>>
>> If we leave the error, maybe a more explanatory message would be helpful?
>> (along the lines of what I posted earlier in this thread, which may be
>> too verbose)
> 
> The message in that patch suggested removing the clobber and hoping for
> the best, which IMO is bad advice.  If the current message doesn't make
> it clear enough that changing the sp is not allowed, how about:
> 
>      inline %<asm%> statements must not change the value of the stack pointer
> 
> ?
> 

Yes, things could be easy, but, mot the closer one looks, the more complicated
they get...

Even pushing a value on the stack and popping it again in the _same_ asm statement
is dangerous with red-zone targets. Maybe that would be also good to add as an advice?


Bernd.

> Thanks,
> Richard
> 
>      
>