mstorsjo wrote:
> Ping @MaskRay
>
> Changes in the force-push:
>
> * rebased on top of main;
> * moved `defaultsToMsStruct` to `ASTContext` since it also depends on
> auxiliary target.
Please update the subject of the PR - this doesn't stub out the attribute, it
implements it, if I understan
DanShaders wrote:
Ping @MaskRay
Changes in the force-push:
- rebased on top of main;
- moved `defaultsToMsStruct` to `ASTContext` since it also depends on auxiliary
target.
https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
https://github.com/DanShaders updated
https://github.com/llvm/llvm-project/pull/71148
>From 39f27a9093edd1b034f193c721b76e85e790693a Mon Sep 17 00:00:00 2001
From: Dan Klishch
Date: Fri, 3 Nov 2023 21:18:06 -0400
Subject: [PATCH] [clang] Stub out gcc_struct attribute
This commit implements gcc
rjmccall wrote:
> @rjmccall Re asking GCC developers about gcc_struct on non-x86:
> https://gcc.gnu.org/pipermail/gcc/2024-January/243154.html. Either GCC devs
> aren't really worried about this or I can't properly write emails (what's
> totally possible).
Alright, well, we tried. I think ro
DanShaders wrote:
@rjmccall Re asking GCC developers about gcc_struct on non-x86:
https://gcc.gnu.org/pipermail/gcc/2024-January/243154.html. Either GCC devs
aren't really worried about this or I can't properly write emails (what's
totally possible).
@MaskRay Any chance you can look at this
https://github.com/DanShaders updated
https://github.com/llvm/llvm-project/pull/71148
>From d0484b032b13109a226c088f18cf0ccd6026bceb Mon Sep 17 00:00:00 2001
From: Dan Klishch
Date: Fri, 3 Nov 2023 21:18:06 -0400
Subject: [PATCH] [clang] Stub out gcc_struct attribute
This commit implements gcc
DanShaders wrote:
@MaskRay Bump
https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/DanShaders updated
https://github.com/llvm/llvm-project/pull/71148
>From b26dd88d456cff7f412cdff6126d0e48454d9eb9 Mon Sep 17 00:00:00 2001
From: Dan Klishch
Date: Fri, 3 Nov 2023 21:18:06 -0400
Subject: [PATCH] [clang] Stub out gcc_struct attribute
This commit implements gcc
mstorsjo wrote:
> Right, I'd just like to make sure that we're not deepening a divergence here.
> It would be good to get agreement from the GCC devs that they think
> `ms_struct` probably ought to do something on e.g. ARM MinGW targets and that
> they consider this a bug (in a feature that th
rjmccall wrote:
Right, I'd just like to make sure that we're not deepening a divergence here.
It would be good to get agreement from the GCC devs that they think `ms_struct`
probably ought to do something on e.g. ARM MinGW targets and that they consider
this a bug (in a feature that they may
DanShaders wrote:
Hmm, but `ms_struct` has been accepted on non-x86 for at least 5 years now. The
PR merely adds a per-structure opt out toggle for it and, honestly, I don't see
how GCC might implement it differently.
https://github.com/llvm/llvm-project/pull/71148
___
rjmccall wrote:
> @rjmccall The problem will arise only if GCC implements support for MSVC C++
> ABI and decides that there is a better way to implement `gcc_struct`. Since,
> AFAIC, MSVC-compatibility for GCC is not even planned, it's unlikely anybody
> there will have strong opinions on this
DanShaders wrote:
@rjmccall
The problem will arise only if GCC implements support for MSVC C++ ABI and
decides that there is a better way to implement `gcc_struct`. Since, AFAIC,
MSVC-compatibility for GCC is not even planned, it's unlikely anybody there
will have strong opinions on this. Yet
rjmccall wrote:
It sounds like we're talking about extending the behavior of these attributes,
and I'd like to make sure that whatever we do here is acceptable to GCC. That
is, if we're going to start accepting these attributes on new targets which GCC
doesn't currently support, we should mak
mstorsjo wrote:
> Okay, @mstorsjo @MaskRay, what is the way forward?
I'm totally not authoritative for these things, but...
> Am I right that, as for the user-facing changes, `[[gcc_struct]]` cancelling
> implicit `-mms-bitfilds` on MinGW is fine
Sounds quite fine for me
> and silently ignor
DanShaders wrote:
Okay, @mstorsjo @MaskRay, what is the way forward?
Am I right that, as for the user-facing changes, `[[gcc_struct]]` cancelling
implicit `-mms-bitfilds` on MinGW is fine and silently ignoring
`-m{no-}ms-bitfields` on `windows-msvc` is not? When exactly should we disallow
`-m
mstorsjo wrote:
> One more thing. Re binary compatibility concerns: `-mno-ms-bitfields` on
> MinGW is an equally-sized footgun as on MSVC. Without proper header
> annotation with `#pragma ms_struct on`, either of them will silently make an
> ABI mismatch. However, for some reason, supporting `
DanShaders wrote:
One more thing. Re binary compatibility concerns: `-mno-ms-bitfields` on MinGW
is an equally-sized footgun as on MSVC. Without proper header annotation with
`#pragma ms_struct on`, either of them will silently make an ABI mismatch.
However, for some reason, supporting `-mno-m
DanShaders wrote:
> Therefore I don't think it's too relevant to implement -mno-ms-bitfields for
> MSVC targets (as I guess it could open a huge can of worms).
I want to only alter the layout of explicit fields in the class with the future
MSVC `-mno-ms-bitfields` implementation. This is alway
DanShaders wrote:
Yes, I know by now :) But this requires using the same type for all the
bit-fields which might lead to unnecessary casts in the algorithm itself. And
the other case is not as easy to fix.
https://github.com/llvm/llvm-project/pull/71148
mstorsjo wrote:
> Microsoft bit-field layout didn't break an overly-specific regression test
> but rendered unusable double to string conversion. The culprit was the
> following snippet:
>
> ```c++
> union Extractor {
> double value;
> struct {
> bool sign : 1;
> u32 exponent : 11;
mstorsjo wrote:
> `-mms-bitfields` is a GCC x86 specific option (`aarch64-linux-gnu-gcc
> -mms-bitfields -xc /dev/null -E` => `error: unrecognized command-line option
> ‘-mms-bitfields’`).
While it is implemented as an x86 specific option in GCC right now, that
doesn't mean that it only is su
DanShaders wrote:
@MaskRay
> I am now confused by the subject "[clang] Stub out gcc_struct attribute".
The user-facing change is `[[gcc_struct]]` attribute implemented for Itanium
C++ ABI. Better handling of `-mms-bitfields` is just a byproduct. I agree that
commit message should be more spe
ADKaster wrote:
> SerenityOS is an OS, independent from Windows. What does porting it to
> Windows mean? Build some SerenityOS components on Windows targeting the PE
> object file format?
In this case, Dan is referring to future plans to port the Ladybird browser to
x86_64-windows-msvc. The b
MaskRay wrote:
I am now confused by the subject "[clang] Stub out gcc_struct attribute".
Do you mean "Implement gcc_struct attribute"? But the description isn't clear
that this patch changes `-mms-bitfields` and `ms_struct` to be respected for
windows-msvc targets.
SerenityOS is an OS, indepen
DanShaders wrote:
@MaskRay
> It does not make sense for MSVC.
Let me share a bit of background here. While porting SerenityOS's libraries to
Windows and, specifically, to `x86_64-pc-windows-msvc`, we discovered a few
tests that were mysteriously failing. It turned out that the change in beha
https://github.com/MaskRay requested changes to this pull request.
.
https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
MaskRay wrote:
`-mms-bitfields` is a GCC x86 specific option (`aarch64-linux-gnu-gcc
-mms-bitfields -xc /dev/null -E` => `error: unrecognized command-line option
‘-mms-bitfields’`).
It does not make sense for MSVC.
Currently, Driver defaults to pass -mms-bitfields to cc1 for `windows-gnu`
trip
DanShaders wrote:
@MaskRay, I figured the alternative solution with the completely decoupled
driver/frontend flags might actually be better than my initial approach. The
only issue I see with it is that it requires
`s/-mms-bitfields/-fms-layout-compatibility=microsoft/g` in quite a large
numb
https://github.com/DanShaders updated
https://github.com/llvm/llvm-project/pull/71148
>From 1033441efd9e790bdb027824ffa1986cd09c06f2 Mon Sep 17 00:00:00 2001
From: Dan Klishch
Date: Fri, 3 Nov 2023 21:18:06 -0400
Subject: [PATCH 1/7] [clang] Stub out gcc_struct attribute
This commit implements
https://github.com/DanShaders updated
https://github.com/llvm/llvm-project/pull/71148
>From 1033441efd9e790bdb027824ffa1986cd09c06f2 Mon Sep 17 00:00:00 2001
From: Dan Klishch
Date: Fri, 3 Nov 2023 21:18:06 -0400
Subject: [PATCH 1/6] [clang] Stub out gcc_struct attribute
This commit implements
https://github.com/DanShaders edited
https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -4393,15 +4393,15 @@ def : Joined<["-"], "mmacosx-version-min=">,
Group, Alias;
def mms_bitfields : Flag<["-"], "mms-bitfields">, Group,
Visibility<[ClangOption, CC1Option]>,
- HelpText<"Set the default structure layout to be compatible with the
Microsoft compiler stan
https://github.com/MaskRay edited
https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -4393,15 +4393,15 @@ def : Joined<["-"], "mmacosx-version-min=">,
Group, Alias;
def mms_bitfields : Flag<["-"], "mms-bitfields">, Group,
Visibility<[ClangOption, CC1Option]>,
- HelpText<"Set the default structure layout to be compatible with the
Microsoft compiler stan
@@ -3618,6 +3618,13 @@ void CompilerInvocationBase::GenerateLangArgs(const
LangOptions &Opts,
GenerateArg(Consumer, OPT_fcxx_abi_EQ,
TargetCXXABI::getSpelling(*Opts.CXXABI));
+ if (Opts.MSBitfields.has_value()) {
+if (Opts.MSBitfields.value())
+
rjmccall wrote:
I agree with the Sema/AST-level LGTM (but also don't feel comfortable approving
the driver changes)
https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/
https://github.com/AaronBallman commented:
The attribute bits LGTM, but I added some more reviewers to double-check the
driver changes.
https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lis
https://github.com/DanShaders updated
https://github.com/llvm/llvm-project/pull/71148
>From 5fb6768149bf2b4e7d74c4874e17dbf4e0e656b7 Mon Sep 17 00:00:00 2001
From: Dan Klishch
Date: Fri, 3 Nov 2023 21:18:06 -0400
Subject: [PATCH 1/5] [clang] Stub out gcc_struct attribute
This commit implements
@@ -3643,7 +3643,14 @@ def CFGuard : InheritableAttr,
TargetSpecificAttr {
def MSStruct : InheritableAttr {
let Spellings = [GCC<"ms_struct">];
let Subjects = SubjectList<[Record]>;
- let Documentation = [Undocumented];
+ let Documentation = [MSStructDocs];
+ let Simple
@@ -7482,3 +7482,26 @@ generation of the other destruction cases, optimizing
the above `foo.destroy` to
}];
}
+
+def MSStructDocs : Documentation {
+ let Category = DocCatDecl;
+ let Content = [{
+The ``ms_struct`` and ``gcc_struct`` attributes request the compiler to ent
DanShaders wrote:
@rjmccall Would you mind merging this then? (I don't have write access)
https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commit
@@ -5031,7 +5031,12 @@ void RecordDecl::completeDefinition() {
/// This which can be turned on with an attribute, pragma, or the
/// -mms-bitfields command-line option.
bool RecordDecl::isMsStruct(const ASTContext &C) const {
- return hasAttr() || C.getLangOpts().MSBitfields =
https://github.com/DanShaders edited
https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/DanShaders edited
https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/DanShaders edited
https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -5031,7 +5031,12 @@ void RecordDecl::completeDefinition() {
/// This which can be turned on with an attribute, pragma, or the
/// -mms-bitfields command-line option.
bool RecordDecl::isMsStruct(const ASTContext &C) const {
- return hasAttr() || C.getLangOpts().MSBitfields =
@@ -5031,7 +5031,12 @@ void RecordDecl::completeDefinition() {
/// This which can be turned on with an attribute, pragma, or the
/// -mms-bitfields command-line option.
bool RecordDecl::isMsStruct(const ASTContext &C) const {
- return hasAttr() || C.getLangOpts().MSBitfields =
@@ -5031,7 +5031,12 @@ void RecordDecl::completeDefinition() {
/// This which can be turned on with an attribute, pragma, or the
/// -mms-bitfields command-line option.
bool RecordDecl::isMsStruct(const ASTContext &C) const {
- return hasAttr() || C.getLangOpts().MSBitfields =
https://github.com/DanShaders updated
https://github.com/llvm/llvm-project/pull/71148
>From 5fb6768149bf2b4e7d74c4874e17dbf4e0e656b7 Mon Sep 17 00:00:00 2001
From: Dan Klishch
Date: Fri, 3 Nov 2023 21:18:06 -0400
Subject: [PATCH 1/4] [clang] Stub out gcc_struct attribute
This commit implements
https://github.com/DanShaders edited
https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -5031,7 +5031,12 @@ void RecordDecl::completeDefinition() {
/// This which can be turned on with an attribute, pragma, or the
/// -mms-bitfields command-line option.
bool RecordDecl::isMsStruct(const ASTContext &C) const {
- return hasAttr() || C.getLangOpts().MSBitfields =
@@ -5031,7 +5031,12 @@ void RecordDecl::completeDefinition() {
/// This which can be turned on with an attribute, pragma, or the
/// -mms-bitfields command-line option.
bool RecordDecl::isMsStruct(const ASTContext &C) const {
- return hasAttr() || C.getLangOpts().MSBitfields =
@@ -7255,20 +7255,27 @@ void Sema::CheckCompletedCXXClass(Scope *S,
CXXRecordDecl *Record) {
CheckCompletedMemberFunction(MD);
}
- // ms_struct is a request to use the same ABI rules as MSVC. Check
- // whether this class uses any C++ features that are implemented
https://github.com/DanShaders updated
https://github.com/llvm/llvm-project/pull/71148
>From 5fb6768149bf2b4e7d74c4874e17dbf4e0e656b7 Mon Sep 17 00:00:00 2001
From: Dan Klishch
Date: Fri, 3 Nov 2023 21:18:06 -0400
Subject: [PATCH 1/3] [clang] Stub out gcc_struct attribute
This commit implements
@@ -998,6 +998,9 @@ def warn_npot_ms_struct : Warning<
"data types with sizes that aren't a power of two">,
DefaultError, InGroup;
+def err_itanium_layout_unimplemented : Error<
+ "sorry, Itanium-compatible layout is unimplemented for the current C++ ABI">;
--
@@ -302,6 +302,11 @@ Attribute Changes in Clang
to reduce the size of the destroy functions for coroutines which are known to
be destroyed after having reached the final suspend point.
+- On targets with C++ ABI other than Microsoft, Clang now supports
eri
@@ -302,6 +302,11 @@ Attribute Changes in Clang
to reduce the size of the destroy functions for coroutines which are known to
be destroyed after having reached the final suspend point.
+- On targets with C++ ABI other than Microsoft, Clang now supports
+ ``[[gnu:gcc_struc
https://github.com/erichkeane commented:
Docs need work, code is fine from what I can tell, but @efriedma-quic or
@rjmccall need to take a look at the ABI components.
https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
cfe-com
https://github.com/erichkeane edited
https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/DanShaders updated
https://github.com/llvm/llvm-project/pull/71148
>From 5fb6768149bf2b4e7d74c4874e17dbf4e0e656b7 Mon Sep 17 00:00:00 2001
From: Dan Klishch
Date: Fri, 3 Nov 2023 21:18:06 -0400
Subject: [PATCH 1/2] [clang] Stub out gcc_struct attribute
This commit implements
@@ -3635,6 +3635,13 @@ def MSStruct : InheritableAttr {
let SimpleHandler = 1;
}
+def GCCStruct : InheritableAttr {
+ let Spellings = [GCC<"gcc_struct">];
+ let Subjects = SubjectList<[Record]>;
+ let Documentation = [Undocumented];
erichkeane wrote:
Doc
https://github.com/erichkeane edited
https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/erichkeane commented:
This needs attribute documentation and a release note.
Additionally, I think we should prohibit using this in Microsoft mode (that is,
diagnose that) rather than having it be a no-op. We can enable it in the
future, but if we permit it, folks will assu
https://github.com/DanShaders updated
https://github.com/llvm/llvm-project/pull/71148
>From 19124226ee4d08a8025f1e6b61d750096e13ee75 Mon Sep 17 00:00:00 2001
From: Dan Klishch
Date: Fri, 3 Nov 2023 21:18:06 -0400
Subject: [PATCH] [clang] Stub out gcc_struct attribute
This commit implements gcc
https://github.com/DanShaders edited
https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/DanShaders edited
https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/DanShaders edited
https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/DanShaders edited
https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/DanShaders edited
https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/DanShaders edited
https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/DanShaders edited
https://github.com/llvm/llvm-project/pull/71148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
72 matches
Mail list logo
