On 11/1/24 10:44 AM, Uros Bizjak wrote:
On Fri, Nov 1, 2024 at 3:51 PM Jeff Law <jeffreya...@gmail.com> wrote:
On 11/1/24 8:45 AM, Uros Bizjak wrote:
On Fri, Nov 1, 2024 at 2:18 PM Jeff Law <jeffreya...@gmail.com> wrote:
On 11/1/24 5:34 AM, Uros Bizjak wrote:
Stack pointer modifications in asm are currently not flagged in
crtl->sp_is_unchanging due to RTX pointer comparison in
notice_stack_pointer_modification_1. Pointer comparison does not detect
that "(reg/v:DI 7 sp [ current_stack_pointer ])" and "(reg/f:DI 7 sp)"
RTXes actually all correspond to the same stack pointer register.
Due to the above omission, the compiler does not detect that asm RTX
manipulates stack pointer in the following construct:
But how did you get two distinct RTXs for the stack pointer? That's not
supposed to happen IIRC.
Please see the testcase in the patch:
+register unsigned long current_stack_pointer asm ("%rsp");
+#define ASM_CALL_CONSTRAINT "+r" (current_stack_pointer)
[...]
+ asm volatile ("pushfq; push %1; pop %0; popfq"
+ : "=r" (y), ASM_CALL_CONSTRAINT
+ : "e" (-1));
When compiled for x86_64, the stack pointer RTX that is different from
the generic stack pointer RTX is created.
If we're allowing that, then I suspect there's a good number of places
that will ultimately need to be fixed. It's baked in pretty deep that
there is only one stack pointer.
grepping for "== stack_pointer_rtx" in gcc/ and its subdirectories
returns 178 hits, which doesn't look that bad.
But if we allow this case, then we probably need to fix the frame
pointer, hard frame pointer and arg pointer cases too.
So I think we need to conclude whether or not the testcase is valid or
not first.
This is what x86 linux kernel uses (arch/x86/include/asm/asm.h) as a
scheduling barrier to prevent asm from being scheduled before stack
frame is constructed:
Just because the kernel does it, doesn't make it right :-)
~~~~~~~~~~~~~~
OTOH, declaring and using %rsp as a global register variable doesn't
error out, so one would expect that it would be used throughout the
compiler as an alias to the internal stack pointer. The presented
testcase exposes one place where this assumption doesn't hold.
But maybe it should error out or should be transformed internally to
reference the singleton rtx object.
Jeff
