[PATCH] D87615: [X86] Fix stack alignment on 32-bit Solaris/x86

This revision was automatically updated to reflect the committed changes. Closed by commit rGa9cbe5cf30e3: [X86] Fix stack alignment on 32-bit Solaris/x86 (authored by ro). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87615/new/

[PATCH] D87615: [X86] Fix stack alignment on 32-bit Solaris/x86

ro added a comment. In D87615#2278436 , @MaskRay wrote: >> This patch avoid the issue by defaulting to -mstackrealign, just like gcc. > > This sentence from the description should be removed. Sure: I usually review and eventually update the description

[PATCH] D87615: [X86] Fix stack alignment on 32-bit Solaris/x86

ro added a comment. In D87615#2278002 , @joerg wrote: > I'm still curious about the source of the vptr diff, but that's a minor > question, otherwise. LGTM Thanks. I think I've found what's going on here: the memory ranges are ultimately printed by

[PATCH] D87615: [X86] Fix stack alignment on 32-bit Solaris/x86

MaskRay accepted this revision. MaskRay added a comment. > This patch avoid the issue by defaulting to -mstackrealign, just like gcc. This sentence from the description should be removed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87615/new/

[PATCH] D87615: [X86] Fix stack alignment on 32-bit Solaris/x86

joerg accepted this revision. joerg added a comment. This revision is now accepted and ready to land. I'm still curious about the source of the vptr diff, but that's a minor question, otherwise. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D87615: [X86] Fix stack alignment on 32-bit Solaris/x86

RKSimon added a comment. @joerg Are you alright with this now? If so please can you accept it to unblock it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87615/new/ https://reviews.llvm.org/D87615 ___

[PATCH] D87615: [X86] Fix stack alignment on 32-bit Solaris/x86

efriedma accepted this revision. efriedma added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87615/new/ https://reviews.llvm.org/D87615 ___ cfe-commits mailing list

[PATCH] D87615: [X86] Fix stack alignment on 32-bit Solaris/x86

ro added a comment. In D87615#2275045 , @efriedma wrote: > Also, it would be nice to have some regression test coverage; add a Solaris > RUN line to llvm/test/CodeGen/X86/stack-align2.ll ? Sure, done in the updated patch. I'd checked that if `FAIL`s

[PATCH] D87615: [X86] Fix stack alignment on 32-bit Solaris/x86

ro updated this revision to Diff 292153. ro added a comment. - Rely on `stackAlignment` default for 32-bit Solaris/x86 - Handle Solaris in `llvm/test/CodeGen/X86/stack-align2.ll` Tested on `amd64-pc-solaris2.11` and `x86_64-pc-linux-gnu`. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D87615: [X86] Fix stack alignment on 32-bit Solaris/x86

efriedma added a comment. Also, it would be nice to have some regression test coverage; add a Solaris RUN line to llvm/test/CodeGen/X86/stack-align2.ll ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87615/new/ https://reviews.llvm.org/D87615

[PATCH] D87615: [X86] Fix stack alignment on 32-bit Solaris/x86

efriedma added inline comments. Comment at: llvm/lib/Target/X86/X86Subtarget.cpp:270 + else if (isTargetSolaris() && !In64BitMode) +stackAlignment = Align(4); stackAlignment is initialized to 4 in the header, so `stackAlignment = Align(4)` here is a

[PATCH] D87615: [X86] Fix stack alignment on 32-bit Solaris/x86

ro updated this revision to Diff 291837. ro added a reviewer: vitalybuka. ro added a comment. Herald added a project: Sanitizers. Herald added a subscriber: Sanitizers. Allow for whitespace differences in `vptr.cpp`. Tested on `amd64-pc-solaris2.11` with the previous `-mstackrealign` patch and

[PATCH] D87615: [X86] Fix stack alignment on 32-bit Solaris/x86

ro added a subscriber: vitalybuka. ro added a comment. In D87615#2273427 , @ro wrote: > > Tested on `amd64-pc-solaris2.11`. However, compared to the `-mstackrealign` > version > there's one regression that I still need to investigate: > >

[PATCH] D87615: [X86] Fix stack alignment on 32-bit Solaris/x86

ro updated this revision to Diff 291831. ro retitled this revision from "[clang][Driver] Force stack realignment on 32-bit Solaris/x86" to "[X86] Fix stack alignment on 32-bit Solaris/x86". ro added a comment. Herald added subscribers: llvm-commits, hiraditya. Herald added a project: LLVM. Set