[PATCH] D60748: Fix i386 struct and union parameter alignment

rjmccall added a comment. In D60748#1925907 , @LiuChen3 wrote: > In D60748#1924794 , @rjmccall wrote: > > > Oh, I see you just updated your patch months ago without ever mentioning > > that it was ready for review.

[PATCH] D60748: Fix i386 struct and union parameter alignment

LiuChen3 added a comment. In D60748#1924794 , @rjmccall wrote: > Oh, I see you just updated your patch months ago without ever mentioning that > it was ready for review. > > It sounds to me like GCC retroactively added a switch specifying which > version

[PATCH] D60748: Fix i386 struct and union parameter alignment

rjmccall added a comment. Oh, I see you just updated your patch months ago without ever mentioning that it was ready for review. It sounds to me like GCC retroactively added a switch specifying which version of the ABI to follow on this point, somewhat confusingly called `-malign-data`. That'

[PATCH] D60748: Fix i386 struct and union parameter alignment

LiuChen3 updated this revision to Diff 250483. LiuChen3 added a comment. Herald added a subscriber: arichardson. rebase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60748/new/ https://reviews.llvm.org/D60748 Files: clang/lib/CodeGen/TargetInfo.cpp clang/test/CodeGen/x86_32-align-

[PATCH] D60748: Fix i386 struct and union parameter alignment

LiuChen3 added a comment. Ping? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60748/new/ https://reviews.llvm.org/D60748 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60748: Fix i386 struct and union parameter alignment

LiuChen3 added a comment. In D60748#1681178 , @kib wrote: > In fact, can we have an option controlling this ? Does it have anything to > do with -malign-data gcc switch ? > > We do want to be able to optionally generate code ABI-compatible with modern >

[PATCH] D60748: Fix i386 struct and union parameter alignment

kib added a comment. In fact, can we have an option controlling this ? Does it have anything to do with -malign-data gcc switch ? We do want to be able to optionally generate code ABI-compatible with modern gcc, per user discretion. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60748

[PATCH] D60748: Fix i386 struct and union parameter alignment

emaste added a comment. In D60748#1499756 , @dim wrote: > Please also exclude FreeBSD from these changes, since we care a lot about > backwards compatibility, and specifically about alignment requirements. (We > have run into many issues in our ports co

[PATCH] D60748: Fix i386 struct and union parameter alignment

LiuChen3 updated this revision to Diff 220793. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60748/new/ https://reviews.llvm.org/D60748 Files: clang/lib/CodeGen/TargetInfo.cpp clang/test/CodeGen/x86_32-align-linux.c clang/test/CodeGen/x86_32-align-linux.cpp clang/test/CodeGen/x86

[PATCH] D60748: Fix i386 struct and union parameter alignment

wxiao3 added a comment. @RKSimon I'm busy with other stuff and my colleague: LiuChen3 will help finish the work. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60748/new/ https://reviews.llvm.org/D60748 ___ cfe-commi

[PATCH] D60748: Fix i386 struct and union parameter alignment

RKSimon requested changes to this revision. RKSimon added a comment. This revision now requires changes to proceed. In D60748#1524443 , @wxiao3 wrote: > Thanks for the information! > We have reverted the patch and will resubmit it when we have a complete

[PATCH] D60748: Fix i386 struct and union parameter alignment

wxiao3 added a comment. Thanks for the information! We have reverted the patch and will resubmit it when we have a complete fix. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60748/new/ https://reviews.llvm.org/D60748 __

[PATCH] D60748: Fix i386 struct and union parameter alignment

pengfei reopened this revision. pengfei added a comment. This revision is now accepted and ready to land. Reverted by https://reviews.llvm.org/rL362186 Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60748/new/ https://reviews.llvm.org/D60748 ___

[PATCH] D60748: Fix i386 struct and union parameter alignment

jyknight added a comment. I don't think this was correct (where by "correct", there, I mean "what GCC does", as this patch is intended to match GCC behavior). I think this change may well break more cases than it fixes, so IMO, this should be reverted, until it's implemented properly. Consider

[PATCH] D60748: Fix i386 struct and union parameter alignment

joerg added inline comments. Comment at: lib/CodeGen/TargetInfo.cpp:1501 +// +// Exclude other System V OS (e.g Darwin, PS4 and FreeBSD) since we don't +// want to spend any effort dealing with the ramifications of ABI breaks. krytarowski wrote: > Dar

[PATCH] D60748: Fix i386 struct and union parameter alignment

krytarowski added subscribers: mgorny, joerg. krytarowski added inline comments. Comment at: lib/CodeGen/TargetInfo.cpp:1501 +// +// Exclude other System V OS (e.g Darwin, PS4 and FreeBSD) since we don't +// want to spend any effort dealing with the ramifications of A

[PATCH] D60748: Fix i386 struct and union parameter alignment

This revision was automatically updated to reflect the committed changes. Closed by commit rC361934: [X86] Fix i386 struct and union parameter alignment (authored by pengfei, committed by ). Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60748/new/ https://reviews.

[PATCH] D60748: Fix i386 struct and union parameter alignment

rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Yes, LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60748/new/ https://reviews.llvm.org/D60748 ___ cfe-commits mailing list cfe

[PATCH] D60748: Fix i386 struct and union parameter alignment

wxiao3 added a comment. Ok for merge now? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60748/new/ https://reviews.llvm.org/D60748 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c

[PATCH] D60748: Fix i386 struct and union parameter alignment

wxiao3 added a comment. Yes, the ABI bug will cause SEGV in Linux where a lot of libraries are built by GCC. I have restricted the fix to Linux only in the latest revision. Any other comments? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60748/new/ https://reviews.llvm.org/D60748

[PATCH] D60748: Fix i386 struct and union parameter alignment

wxiao3 updated this revision to Diff 199360. wxiao3 edited the summary of this revision. Herald added a subscriber: krytarowski. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60748/new/ https://reviews.llvm.org/D60748 Files: lib/CodeGen/TargetInfo.cpp test/CodeGen/x86_32-align-linux.

[PATCH] D60748: Fix i386 struct and union parameter alignment

dim added a comment. In fact, it is probably better to turn the OS check around, e.g. *only* increase the alignment for Linux, and nowhere else. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60748/new/ https://reviews.llvm.org/D60748 ___ c

[PATCH] D60748: Fix i386 struct and union parameter alignment

dim added subscribers: emaste, dim. dim added a comment. Please also exclude FreeBSD from these changes, since we care a lot about backwards compatibility, and specifically about alignment requirements. (We have run into many issues in our ports collection where upstream assumes everything is

[PATCH] D60748: Fix i386 struct and union parameter alignment

wxiao3 added a comment. Any other comments? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60748/new/ https://reviews.llvm.org/D60748 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe

[PATCH] D60748: Fix i386 struct and union parameter alignment

wxiao3 updated this revision to Diff 199231. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60748/new/ https://reviews.llvm.org/D60748 Files: lib/CodeGen/TargetInfo.cpp test/CodeGen/vector.c test/CodeGen/x86_32-arguments-darwin.c test/CodeGen/x86_32-arguments-linux.c test/CodeGe

[PATCH] D60748: Fix i386 struct and union parameter alignment

wxiao3 updated this revision to Diff 199232. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60748/new/ https://reviews.llvm.org/D60748 Files: lib/CodeGen/TargetInfo.cpp test/CodeGen/x86_32-align-linux.c test/CodeGen/x86_32-arguments-linux.c Index: test/CodeGen/x86_32-arguments-linu

[PATCH] D60748: Fix i386 struct and union parameter alignment

wxiao3 added a comment. Ok, I have excluded Darwin and PS4 for the changes. The fix mainly targets at Linux so that we can compile a project with parts by GCC and parts by LLVM given that they follow the same ABI. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60748/new/ https://reviews

[PATCH] D60748: Fix i386 struct and union parameter alignment

wxiao3 updated this revision to Diff 196967. wxiao3 edited the summary of this revision. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60748/new/ https://reviews.llvm.org/D60748 Files: lib/CodeGen/TargetInfo.cpp test/CodeGen/x86_32-align-linux.c test/CodeGen/x86_32-arguments-linux.

[PATCH] D60748: Fix i386 struct and union parameter alignment

dexonsmith added a comment. In D60748#1472829 , @rjmccall wrote: > I suspect Darwin also doesn't want to take this. We care very little about > 32-bit Intel, and part of caring very little is not wanting to spend any > effort dealing with the ramificati

[PATCH] D60748: Fix i386 struct and union parameter alignment

rjmccall added a subscriber: dexonsmith. rjmccall added a comment. I suspect Darwin also doesn't want to take this. We care very little about 32-bit Intel, and part of caring very little is not wanting to spend any effort dealing with the ramifications of ABI breaks. That would apply both to t

[PATCH] D60748: Fix i386 struct and union parameter alignment

rnk added a comment. This is, obviously, an ABI break. I think Sony would probably want you to preserve the existing behavior of intentionally underaligning such byval parameters for PS4 targets. +@rjmccall in case he has other ABI thoughts. Comment at: lib/CodeGen/TargetInfo

[PATCH] D60748: Fix i386 struct and union parameter alignment

wxiao3 created this revision. wxiao3 added reviewers: annita.zhang, LuoYuanke, smaslov, hjl.tools, RKSimon, rnk, andreadb. wxiao3 added a project: clang. Herald added a subscriber: cfe-commits. According to i386 System V ABI 2.1: Structures and unions assume the alignment of their most strictly a