Inline assembly with stack-pointer clobbers not handled as well as it could be -- easy fix

[pskocik]

Although GCC complains about stack pointer clobbers from inline assembly (declares to have deprecated them), it actually supports them better than Clang by:

• forcing a frame pointer
• avoiding the redzone
• restoring the stack pointer based on the frame pointer

This allows stack-allocating inline assembly to work (and stackfreeing inline assembly: as long as just allocated stack space is freed and spills made by the compiler aren't jeopardized) in addition to making inline assembly such as "pushf; pop ..." harmless inside of code that might otherwise want to store a local in the first slot of the red zone (see https://godbolt.org/z/rv3aoK3x8 where GCC has correct codegen but Clang doesn't (based on https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108799 by listing the rsp in clobbers)).

I think for Clang, this would be an easy fix: simply treat frames with opaque stack pointer adjustments as if they had variably-sized objects:

diff --git a/llvm/include/llvm/CodeGen/MachineFrameInfo.h b/llvm/include/llvm/CodeGen/MachineFrameInfo.h
index 7d11d63d4..5cc2045e4 100644
--- a/llvm/include/llvm/CodeGen/MachineFrameInfo.h
+++ b/llvm/include/llvm/CodeGen/MachineFrameInfo.h
@@ -352,7 +352,7 @@ public:
   /// This method may be called any time after instruction
   /// selection is complete to determine if the stack frame for this function
   /// contains any variable sized objects.
-  bool hasVarSizedObjects() const { return HasVarSizedObjects; }
+  bool hasVarSizedObjects() const { return HasVarSizedObjects || HasOpaqueSPAdjustment; }

This simple change makes the above-listed pushf;pop ... code example no longer crash in addition to making code like:

void push(void){ __asm volatile("push $0;" ::: "rsp"); }
void loopy(void){
    long register rsp __asm("rsp");
    void *savedSp; __asm ("mov %1, %0\n" : "=rm"(savedSp): "r"(rsp) );
    for(int i=0;i<3;i++){
        __asm volatile("push $0;" ::: "rsp");
        __asm volatile("push $0;" ::: "rsp");
        asm volatile ("mov %1, %0\n" : "=r"(rsp) : "r"(savedSp));
    }
}

no longer crash (at least with optimization at at least -O1 -- the rsp reading assembly at the start of loopy() actually crashes at -O0 on Clang (not GCC) due to #61897).

[llvmbot]

@llvm/issue-subscribers-clang-codegen

Although GCC complains about stack pointer clobbers from inline assembly (declares to have deprecated them), it actually supports them better than Clang by:

• forcing a frame pointer
• avoiding the redzone
• restoring the stack pointer based on the frame pointer

This allows stack-allocating inline assembly to work (and stackfreeing inline assembly: as long as just allocated stack space is freed and spills made by the compiler aren't jeopardized) in addition to making inline assembly such as "pushf; pop ..." harmless inside of code that might otherwise want to store a local in the first slot of the red zone (see https://godbolt.org/z/rv3aoK3x8 where GCC has correct codegen but Clang doesn't (based on https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108799 by listing the rsp in clobbers)).

I think for Clang, this would be an easy fix: simply treat frames with opaque stack pointer adjustments as if they had variably-sized objects:

diff --git a/llvm/include/llvm/CodeGen/MachineFrameInfo.h b/llvm/include/llvm/CodeGen/MachineFrameInfo.h
index 7d11d63d4..5cc2045e4 100644
--- a/llvm/include/llvm/CodeGen/MachineFrameInfo.h
+++ b/llvm/include/llvm/CodeGen/MachineFrameInfo.h
@@ -352,7 +352,7 @@ public:
   /// This method may be called any time after instruction
   /// selection is complete to determine if the stack frame for this function
   /// contains any variable sized objects.
-  bool hasVarSizedObjects() const { return HasVarSizedObjects; }
+  bool hasVarSizedObjects() const { return HasVarSizedObjects || HasOpaqueSPAdjustment; }

This simple change makes the above-listed pushf;pop ... code example no longer crash in addition to making code like:

void push(void){ __asm volatile("push $0;" ::: "rsp"); }
void loopy(void){
    long register rsp __asm("rsp");
    void *savedSp; __asm ("mov %1, %0\n" : "=rm"(savedSp): "r"(rsp) );
    for(int i=0;i<3;i++){
        __asm volatile("push $0;" ::: "rsp");
        __asm volatile("push $0;" ::: "rsp");
        asm volatile ("mov %1, %0\n" : "=r"(rsp) : "r"(savedSp));
    }
}

no longer crash (at least with optimization at at least -O1 -- the rsp reading assembly at the start of loopy() actually crashes at -O0 on Clang (not GCC) due to #61897).