On November 6, 2024 7:27:51 AM PST, Uros Bizjak <ubiz...@gmail.com> wrote:
>On Wed, Nov 6, 2024 at 11:57 AM Jakub Jelinek <ja...@redhat.com> wrote:
>>
>> On Wed, Nov 06, 2024 at 10:44:54AM +0100, Uros Bizjak wrote:
>> > After some more thinking and considering all recent discussion
>> > (thanks!), I am convinced that a slightly simplified original patch
>> > (attached), now one-liner, is the way to go.
>>
>> > --cut here--> > register unsigned long current_stack_pointer asm ("%rsp");
>> > #define ASM_CALL_CONSTRAINT "+r" (current_stack_pointer)
>> >
>> > unsigned long baz (void)
>> > {
>> > unsigned long res;
>> >
>> > asm ("pushfq; popq %0" : "=r" (res), ASM_CALL_CONSTRAINT);
>> > return res;
>> > }
>> > --cut here--
>> >
>> > here we inform the compiler that we read and write the rsp, not
>> > necessarily change it, maybe something like "add %rsp, $0". As above,
>> > notice_stack_pointer_modification_1 does not trigger here.
>>
>> While this happened to work, we've discussed several times this isn't
>> something we want to recommend to people.
>> Generally, if inline asm temporarily changes some register and restores it
>> back before returning to C code, there is no need to mention that register
>> in clobbers or in "+r" operand. NAnd the "+r" (current_stack_pointer)
>> says that the stack pointer was or could have been changed, which is wrong,
>> valid inline asm can never change the stack pointer. It can change it
>> temporarily and then restore if it knows what it is doing.
>
>I see. While my solution would fit nicely with the above
>ASM_CALL_CONSTRAINT approach, the approach using ASM_CALL_CONSTRAINT
>is wrong by itself.
>
>Oh, well.
>
>Anyway, I guess "redzone" clobber you proposed does not remove the
>need to use ASM_CALL_CONSTRAINT if we want to confine the "asm with
>call inside" between frame setup/teardown insns? Is it possible to
>invent similar clobber (unimaginatively called "stack", perhaps) that
>would prevent scheduler from moving asm to the wrong place?
>
>Uros.
>
I suggested __builtin_frame_address(0) as an input constraint (which already
works in gcc and clang) and the "red-zone" clobber (new) for this exact reason
(Andrew Pinski, however, summarily closed those BRs.)
Semantically, using the "+r" (rsp) constraint obviously doesn't make any sense,
but we don't care if it is what the gcc community wants to support going
forward, we'll use it, but it really, really needs to be documented as it is
incredibly non-obvious.
