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

[Phoebe Wang via Phabricator via llvm-commits]

pengfei accepted this revision.
pengfei added a comment.
This revision is now accepted and ready to land.

LGTM except for one nit.



================
Comment at: llvm/lib/Target/X86/X86FrameLowering.cpp:1248
+    if (HasRealign)
+      MaxAlign = Align(std::lcm(16, MaxAlign.value()));
+    else
----------------
antangelo wrote:
> pengfei wrote:
> > antangelo wrote:
> > > pengfei wrote:
> > > > pengfei wrote:
> > > > > I think `max` is enough. We don't have no-power-of-2 alginment.
> > > > Where's the `16` request from, ABI?
> > > The 16 byte alignment is from SysV ABI. I believe the latest revision is here https://gitlab.com/x86-psABIs/i386-ABI/-/tree/hjl/x86/master
> > > 
> > > > The end of the input argument area shall be aligned on a 16 (32 or 64, if `__m256` or `__m512` is passed on stack) byte boundary
> > My understanding is psABI just defines the default calling conversion. I didn't see description about interrupt in the ABI doc.
> > What's the GCC behavior? Is this to match with it?
> GCC aligns the stack to 16 bytes if the interrupt handler makes calls to functions outside of the current compilation unit (I believe this is where the ABI alignment comes into play unless I'm misunderstanding). The goal is to align with GCC's behavior.
Got it, thanks!


================
Comment at: llvm/test/CodeGen/X86/x86-32-intrcc.ll:408-409
 attributes #0 = { nounwind "frame-pointer"="all" }
+attributes #1 = { "no-realign-stack" }
+attributes #2 = { "stackrealign" }
----------------
Add `nounwind` to remove `.cfi*`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151400