The bug is introduced in rGe4ceb5a7bb9b which set the wrong offset from
the stack base. This patch is to fix the bug.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/Target/X86/X86FrameLowering.cpp | ||
---|---|---|
1590 | 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? | |
2327 | ditto. | |
llvm/test/CodeGen/X86/i386-baseptr.ll | ||
77 | The dwarf doesn't need to update together? |
llvm/lib/Target/X86/X86FrameLowering.cpp | ||
---|---|---|
1590 | 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. | |
llvm/test/CodeGen/X86/i386-baseptr.ll | ||
77 | 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. |
llvm/lib/Target/X86/X86FrameLowering.cpp | ||
---|---|---|
1590 | I got regression with below change. It is wired to me. Maybe it is due to the type of SlotSize is unsigned. diff --git a/llvm/lib/Target/X86/X86FrameLowering.cpp b/llvm/lib/Target/X86/X86FrameLowering.cpp index 15f39a509e1a..f3f21f0ac46d 100644 --- a/llvm/lib/Target/X86/X86FrameLowering.cpp +++ b/llvm/lib/Target/X86/X86FrameLowering.cpp @@ -1587,7 +1587,7 @@ void X86FrameLowering::emitPrologue(MachineFunction &MF, MachineInstr::FrameSetup); } BuildStackAlignAND(MBB, MBBI, DL, StackPtr, MaxAlign); - int64_t Offset = -1 * (int64_t)SlotSize; + int64_t Offset = -SlotSize; BuildMI(MBB, MBBI, DL, TII.get(Is64Bit ? X86::PUSH64rmm: X86::PUSH32rmm)) .addReg(ArgBaseReg) .addImm(1) @@ -2324,6 +2324,7 @@ void X86FrameLowering::emitEpilogue(MachineFunction &MF, .addUse(ArgBaseReg) .addImm(1) .addUse(X86::NoRegister) + // .addImm((int64_t)-SlotSize) .addImm((int64_t)SlotSize * -1) .addUse(X86::NoRegister) .setMIFlag(MachineInstr::FrameDestroy); |
llvm/lib/Target/X86/X86FrameLowering.cpp | ||
---|---|---|
1590 | How about -(int64_t)SlotSize? |
llvm/lib/Target/X86/X86FrameLowering.cpp | ||
---|---|---|
1590 | It works. :) I'll adopt this version. |
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?