[PATCH] D151400: [X86] Align stack to 16-bytes on 32-bit with X86_INTR call convention

[Phoebe Wang via Phabricator via llvm-commits]

pengfei added inline comments.


================
Comment at: llvm/lib/Target/X86/X86FrameLowering.cpp:1248
+    if (HasRealign)
+      MaxAlign = Align(std::lcm(16, MaxAlign.value()));
+    else
----------------
I think `max` is enough. We don't have no-power-of-2 alginment.


================
Comment at: llvm/lib/Target/X86/X86FrameLowering.cpp:1248
+    if (HasRealign)
+      MaxAlign = Align(std::lcm(16, MaxAlign.value()));
+    else
----------------
pengfei wrote:
> I think `max` is enough. We don't have no-power-of-2 alginment.
Where's the `16` request from, ABI?


================
Comment at: llvm/test/CodeGen/X86/x86-32-intrcc.ll:10-11
 
 ; Spills eax, putting original esp at +4.
 ; No stack adjustment if declared with no error code
 define x86_intrcc void @test_isr_no_ecode(ptr byval(%struct.interrupt_frame) %frame) {
----------------
This seems conflict with the intention here.


================
Comment at: llvm/test/CodeGen/X86/x86-32-intrcc.ll:35-36
 
 ; Spills eax and ecx, putting original esp at +8. Stack is adjusted up another 4 bytes
 ; before return, popping the error code.
 define x86_intrcc void @test_isr_ecode(ptr byval(%struct.interrupt_frame) %frame, i32 %ecode) {
----------------
ditto.


================
Comment at: llvm/test/CodeGen/X86/x86-32-intrcc.ll:103
   ; CHECK-NEXT: fstpt f80
-  ; CHECK-NEXT: iretl
+  ; CHECK: iretl
 entry:
----------------
This should not be affected.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151400/new/

https://reviews.llvm.org/D151400