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

[Richard Sandiford <richard.sandiford@arm.com>]

(Maybe the discussion has moved on from this already -- sorry if so)

Christophe Lyon <christophe.lyon@linaro.org> writes:
> On Wed, 12 Dec 2018 at 12:21, Thomas Preudhomme
> <thomas.preudhomme@linaro.org> wrote:
>>
>> So my understanding is that the original code (CMSIS library) used to
>> clobber sp because the asm statement was actually changing the sp.
>> That in turn led GCC to try to save and restore sp which is not what
>> CMSIS was expecting to happen. Changing sp without clobber as done now
>> is probably the right solution and r242693 can be reverted. That will
>> remove the failing test.
>>
>
> OK, I read PR52813 too, but I'm not sure to fully understand the new status.
> My understanding is that since this patch was committed, if an asm statement
> clobbers sp, it is now allowed to actually declare it as clobber (this patch
> generates an error in such a case).
> So the user is now expected to lie to the compiler when writing to
> this kind of register (sp, pic register), by not declaring it as "clobber"?

The user isn't expected to lie.  The point is that GCC simply doesn't
support asms that leave the stack pointer with a different value from
before, and IMO never has.  If that happened to work in some cases then
it was purely an accident.

The PRs also show disagreement about what GCC should do for an asm like
that.  The asm in PR52813 temporarily changed the stack pointer and the
bug was that GCC didn't restore the original value afterwards.  The asm
in PR77904 was trying to set the stack pointer to an entirely new value
and the bug was the GCC did restore the original value afterwards,
defeating the point.

This wouldn't be the first time that there's disagreement about what
the behaviour should be.  But IMO we can't support either reliably.
Spilling sp is dangerous in general because we might need the stack
for the reload, or we might accidentally try to reload something else
before restoring the stack pointer.  And continuing with a new sp
provided by the asm could lead to all sorts of problems.  (AIUI, the
point of PR77904 was that it would also be wrong for GCC to set up a
frame pointer and restore the sp from that frame pointer on function
exit.  The new sp value was supposed to survive.  So the answer isn't
simply "use a frame pointer".)

Thanks,
Richard