[PATCH] D146862: [X86] Fix the incorrect displacement for prolog/epilog
LuoYuanke via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 24 20:40:53 PDT 2023
LuoYuanke added inline comments.
================
Comment at: llvm/lib/Target/X86/X86FrameLowering.cpp:1590
BuildStackAlignAND(MBB, MBBI, DL, StackPtr, MaxAlign);
- int64_t Offset = Is64Bit ? -2 * (int64_t)SlotSize : -1 * (int64_t)SlotSize;
+ int64_t Offset = -1 * (int64_t)SlotSize;
BuildMI(MBB, MBBI, DL, TII.get(Is64Bit ? X86::PUSH64rmm: X86::PUSH32rmm))
----------------
pengfei wrote:
> Curious why using 2 before. I was thinking it's related to stack aligement. AFAIK, Windows requires the stack always aligned to 16B on 64-bits.
>
> Is better to directly `int64_t Offset = -SlotSize`?
Because my previous version is to push a callee saved register `push %rbx` as well, and then I realized that it is more complex on unwind support, and I turn to a scratch register which doesn't need to be saved and restore. But I forget to modify the offset. I'm developing a runtime test case to cover this issue.
================
Comment at: llvm/test/CodeGen/X86/i386-baseptr.ll:77
+; CHECK-NEXT: leal -4(%ecx), %esp
; CHECK-NEXT: .cfi_def_cfa %esp, 4
; CHECK-NEXT: retl
----------------
pengfei wrote:
> The dwarf doesn't need to update together?
The dwarf seems correct. The ecx pointer to the base stack, and `esp = ecx - 4` pointer to the offset 4 which is the function return address.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146862/new/
https://reviews.llvm.org/D146862
More information about the llvm-commits
mailing list