[clang] [clang] Handle trivial_abi attribute for Microsoft ABI. (PR #88857)

2024-04-29 Thread Reid Kleckner via cfe-commits 


@@ -1105,6 +1105,11 @@ bool MicrosoftCXXABI::hasMostDerivedReturn(GlobalDecl 
GD) const {
 
 static bool isTrivialForMSVC(const CXXRecordDecl *RD, QualType Ty,
  CodeGenModule ) {
+  // If the record is marked with the trivial_abi attribute, we don't
+  // have to conform to the standard MSVC ABI.
+  if (RD->hasAttr())

rnk wrote:

The pseudo code version is something like:
  bool containsTrivialAbiSubobject(CXXRecordDecl *RD) {
if (RD->hasAttr()) return true;
for (auto B : RD->bases()) {
  if (containsTrivialAbiSubobject(B->getAsCXXRecordDecl()))
return true;
}
for (auto F : RD->fields()) {
  const CXXRecordDecl* FRD = F->getType()->getAsCXXRecordDecl();
  if (FRD && containsTrivialAbiSubobject(FRD))
return true;
}
return false;
  }

A bit of a gross inefficient DFS, with some CXXBaseSpecifier details omitted. 
It might also be worth tracking Visited records to handle fields of the same 
record type without recursing over all subobjects again.

https://github.com/llvm/llvm-project/pull/88857
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Handle trivial_abi attribute for Microsoft ABI. (PR #88857)

2024-04-29 Thread Tobias Hieta via cfe-commits 


@@ -1105,6 +1105,11 @@ bool MicrosoftCXXABI::hasMostDerivedReturn(GlobalDecl 
GD) const {
 
 static bool isTrivialForMSVC(const CXXRecordDecl *RD, QualType Ty,
  CodeGenModule ) {
+  // If the record is marked with the trivial_abi attribute, we don't
+  // have to conform to the standard MSVC ABI.
+  if (RD->hasAttr())

tru wrote:

Happy to add this. Can you point me to an example where we search recursively 
or give some pseudo code example and I should be able to take it from there 
pretty soon. 

https://github.com/llvm/llvm-project/pull/88857
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Handle trivial_abi attribute for Microsoft ABI. (PR #88857)

2024-04-29 Thread Reid Kleckner via cfe-commits 

rnk wrote:

Sorry for the delay, work life does its best to intervene.

https://github.com/llvm/llvm-project/pull/88857
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Handle trivial_abi attribute for Microsoft ABI. (PR #88857)

2024-04-29 Thread Reid Kleckner via cfe-commits 


@@ -1105,6 +1105,11 @@ bool MicrosoftCXXABI::hasMostDerivedReturn(GlobalDecl 
GD) const {
 
 static bool isTrivialForMSVC(const CXXRecordDecl *RD, QualType Ty,
  CodeGenModule ) {
+  // If the record is marked with the trivial_abi attribute, we don't
+  // have to conform to the standard MSVC ABI.
+  if (RD->hasAttr())

rnk wrote:

Yes, I think I do want to block this on the recursive search. Even if it's ugly 
and we ultimately replace it with an explicitly tracked and type trait 
(`containsTrivialAbiSubobject`?), it reduces the number of externally visible 
ABI changes. I know we're saying this is ABI unstable already, but I still 
think fewer breaks is better.

https://github.com/llvm/llvm-project/pull/88857
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Handle trivial_abi attribute for Microsoft ABI. (PR #88857)

2024-04-29 Thread Tobias Hieta via cfe-commits 

tru wrote:

ping on this.


https://github.com/llvm/llvm-project/pull/88857
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Handle trivial_abi attribute for Microsoft ABI. (PR #88857)

2024-04-19 Thread Tobias Hieta via cfe-commits 


@@ -1105,6 +1105,11 @@ bool MicrosoftCXXABI::hasMostDerivedReturn(GlobalDecl 
GD) const {
 
 static bool isTrivialForMSVC(const CXXRecordDecl *RD, QualType Ty,
  CodeGenModule ) {
+  // If the record is marked with the trivial_abi attribute, we don't
+  // have to conform to the standard MSVC ABI.
+  if (RD->hasAttr())

tru wrote:

So if I understand correctly:

- There are additional places in the microsoft code path where we don't handle 
trivial cases without the attribute.
- The glaring issue here with this PR is that if you put a trivial_abi struct 
in a struct it won't be marked as trivial itself since it doesn't search 
recursively.

Seems like there are many more places where we can do better, do we want to 
block this PR on the recursive search? What would the rules for that be? It 
can't just be that if you have a struct with a trivial_abi attribute inside any 
struct the parent should also be considered trivial. Would it be that 
isTrivialForMicrosoft would return true if the current struct passes the checks 
there and only contains a struct with a trivial_abi attribute?

https://github.com/llvm/llvm-project/pull/88857
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Handle trivial_abi attribute for Microsoft ABI. (PR #88857)

2024-04-18 Thread Eli Friedman via cfe-commits 


@@ -1105,6 +1105,11 @@ bool MicrosoftCXXABI::hasMostDerivedReturn(GlobalDecl 
GD) const {
 
 static bool isTrivialForMSVC(const CXXRecordDecl *RD, QualType Ty,
  CodeGenModule ) {
+  // If the record is marked with the trivial_abi attribute, we don't
+  // have to conform to the standard MSVC ABI.
+  if (RD->hasAttr())

efriedma-quic wrote:

Oh, I see, whether the constructors/destructor/etc. are trivial for calls is 
determined recursively; we don't record whether that was because of an explicit 
trivial_abi attribute, or because there just weren't any non-trivial fields.  
So we can't tell the difference here.

It feels sort of weird to treat a struct with a trivial_abi member as somehow 
more trivial than a struct that just contains plain data.

https://github.com/llvm/llvm-project/pull/88857
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Handle trivial_abi attribute for Microsoft ABI. (PR #88857)

2024-04-18 Thread Reid Kleckner via cfe-commits 


@@ -1105,6 +1105,11 @@ bool MicrosoftCXXABI::hasMostDerivedReturn(GlobalDecl 
GD) const {
 
 static bool isTrivialForMSVC(const CXXRecordDecl *RD, QualType Ty,
  CodeGenModule ) {
+  // If the record is marked with the trivial_abi attribute, we don't
+  // have to conform to the standard MSVC ABI.
+  if (RD->hasAttr())

rnk wrote:

> That's not how trivial_abi normally works?

I think it is how it works, based on re-reading the code just now and the test 
case I linked.

To your point, the MSVC-specific code in `canPassInRegisters` is full of bugs, 
but I don't want to ask @tru  to deal with that.

https://github.com/llvm/llvm-project/pull/88857
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Handle trivial_abi attribute for Microsoft ABI. (PR #88857)

2024-04-18 Thread Reid Kleckner via cfe-commits 

https://github.com/rnk requested changes to this pull request.


https://github.com/llvm/llvm-project/pull/88857
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Handle trivial_abi attribute for Microsoft ABI. (PR #88857)

2024-04-18 Thread Reid Kleckner via cfe-commits 

https://github.com/rnk edited https://github.com/llvm/llvm-project/pull/88857
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Handle trivial_abi attribute for Microsoft ABI. (PR #88857)

2024-04-18 Thread Reid Kleckner via cfe-commits 


@@ -1105,6 +1105,11 @@ bool MicrosoftCXXABI::hasMostDerivedReturn(GlobalDecl 
GD) const {
 
 static bool isTrivialForMSVC(const CXXRecordDecl *RD, QualType Ty,
  CodeGenModule ) {
+  // If the record is marked with the trivial_abi attribute, we don't
+  // have to conform to the standard MSVC ABI.
+  if (RD->hasAttr())

rnk wrote:

I've realized this misses a case, consider this a wrapper struct that contains 
a trivial_abi subobject: https://godbolt.org/z/bK6aGv4d7

I spent 15 minutes looking through the things we track for trivial_abi, and 
unfortunately, we don't track this anywhere already, so the best solution I can 
come up with is to do a recursive walk of all the subobjects and look for the 
attribute. :(

https://github.com/llvm/llvm-project/pull/88857
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Handle trivial_abi attribute for Microsoft ABI. (PR #88857)

2024-04-18 Thread Eli Friedman via cfe-commits 

https://github.com/efriedma-quic approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/88857
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Handle trivial_abi attribute for Microsoft ABI. (PR #88857)

2024-04-17 Thread Tobias Hieta via cfe-commits 

tru wrote:

@efriedma-quic are you ok with this change?

https://github.com/llvm/llvm-project/pull/88857
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Handle trivial_abi attribute for Microsoft ABI. (PR #88857)

2024-04-16 Thread Reid Kleckner via cfe-commits 

https://github.com/rnk approved this pull request.

Thanks!

https://github.com/llvm/llvm-project/pull/88857
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Handle trivial_abi attribute for Microsoft ABI. (PR #88857)

2024-04-16 Thread Reid Kleckner via cfe-commits 


@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -std=c++11 -fcxx-exceptions 
-fexceptions -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: %[[STRUCT_TRIVIAL:.*]] = type { ptr }
+struct __attribute__((trivial_abi)) Trivial {
+  int *p;
+  Trivial() : p(0) {}
+  Trivial(const Trivial &) noexcept = default;
+};
+
+// CHECK-LABEL: define{{.*}} i64 @"?retTrivial@@YA?AUTrivial@@XZ"(
+// CHECK: %retval = alloca %[[STRUCT_TRIVIAL]], align 8
+// CHECK: %call = call noundef ptr @"??0Trivial@@QEAA@XZ"(ptr noundef nonnull 
align 8 dereferenceable(8) %retval)
+// CHECK: %coerce.dive = getelementptr inbounds %[[STRUCT_TRIVIAL]], ptr 
%retval, i32 0, i32 0
+// CHECK: %0 = load ptr, ptr %coerce.dive, align 8
+// CHECK: %coerce.val.pi = ptrtoint ptr %0 to i64
+// CHECK: ret i64 %coerce.val.pi
+Trivial retTrivial() {
+  Trivial s;
+  return s;
+}
+

rnk wrote:

I stand corrected, but I think it makes the point that this logic can be 
confusing, and it's worth testing. :)

https://github.com/llvm/llvm-project/pull/88857
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Handle trivial_abi attribute for Microsoft ABI. (PR #88857)

2024-04-16 Thread Tobias Hieta via cfe-commits 

https://github.com/tru updated https://github.com/llvm/llvm-project/pull/88857

>From 08214d87d1a7c83ea25eef3bf18de1568a20a152 Mon Sep 17 00:00:00 2001
From: Tobias Hieta 
Date: Tue, 16 Apr 2024 09:38:53 +0200
Subject: [PATCH] [clang] Handle trivial_abi attribute for Microsoft ABI.

Previously the trivial_abi was ignored for records when targetting
the microsoft abi, the MSVC rules where always enforced to ensure
compatibility with MSVC. This commit changes it to be closer to
the itanium abi when a record is marked with the trivial_abi attribute.

Fixes #87993
---
 clang/docs/ReleaseNotes.rst|  5 
 clang/lib/CodeGen/MicrosoftCXXABI.cpp  |  5 
 clang/test/CodeGenCXX/trivial_abi_msvc.cpp | 31 ++
 3 files changed, 41 insertions(+)
 create mode 100644 clang/test/CodeGenCXX/trivial_abi_msvc.cpp

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 76701dc723b6c3..2b653e5af6f5ae 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -63,6 +63,11 @@ ABI Changes in This Version
   MSVC uses a different mangling for these objects, compatibility is not 
affected.
   (#GH85423).
 
+- Records carrying the trivial_abi attribute are now returned directly in 
registers
+  in more cases when using the Microsoft ABI. It is not possible to pass 
trivial_abi
+  records between MSVC and Clang, so there is no ABI compatibility 
requirement. This
+  is an ABI break with old versions of Clang. (#GH87993)
+
 AST Dumping Potentially Breaking Changes
 
 
diff --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp 
b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
index d38a26940a3cb6..b930913badcd3f 100644
--- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -1105,6 +1105,11 @@ bool MicrosoftCXXABI::hasMostDerivedReturn(GlobalDecl 
GD) const {
 
 static bool isTrivialForMSVC(const CXXRecordDecl *RD, QualType Ty,
  CodeGenModule ) {
+  // If the record is marked with the trivial_abi attribute, we don't
+  // have to conform to the standard MSVC ABI.
+  if (RD->hasAttr())
+return true;
+
   // On AArch64, HVAs that can be passed in registers can also be returned
   // in registers. (Note this is using the MSVC definition of an HVA; see
   // isPermittedToBeHomogeneousAggregate().)
diff --git a/clang/test/CodeGenCXX/trivial_abi_msvc.cpp 
b/clang/test/CodeGenCXX/trivial_abi_msvc.cpp
new file mode 100644
index 00..0af1d63acd1a5e
--- /dev/null
+++ b/clang/test/CodeGenCXX/trivial_abi_msvc.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -std=c++11 -fcxx-exceptions 
-fexceptions -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: %[[STRUCT_TRIVIAL:.*]] = type { ptr }
+struct __attribute__((trivial_abi)) Trivial {
+  int *p;
+  Trivial() : p(0) {}
+  Trivial(const Trivial &) noexcept = default;
+};
+
+// CHECK-LABEL: define{{.*}} i64 @"?retTrivial@@YA?AUTrivial@@XZ"(
+// CHECK: %retval = alloca %[[STRUCT_TRIVIAL]], align 8
+// CHECK: %call = call noundef ptr @"??0Trivial@@QEAA@XZ"(ptr noundef nonnull 
align 8 dereferenceable(8) %retval)
+// CHECK: %coerce.dive = getelementptr inbounds %[[STRUCT_TRIVIAL]], ptr 
%retval, i32 0, i32 0
+// CHECK: %0 = load ptr, ptr %coerce.dive, align 8
+// CHECK: %coerce.val.pi = ptrtoint ptr %0 to i64
+// CHECK: ret i64 %coerce.val.pi
+Trivial retTrivial() {
+  Trivial s;
+  return s;
+}
+
+struct TrivialInstance {
+Trivial instanceMethod();
+static Trivial staticMethod();
+};
+
+// We need to make sure that instanceMethod has a sret return value since 
`this` will always go in the register.
+// CHECK-LABEL: define{{.*}} void 
@"?instanceMethod@TrivialInstance@@QEAA?AUTrivial@@XZ"({{.*}} 
sret(%struct.Trivial{{.*}}
+Trivial TrivialInstance::instanceMethod() { return {}; }
+// CHECK-LABEL: define{{.*}} i64 
@"?staticMethod@TrivialInstance@@SA?AUTrivial@@XZ"(
+Trivial TrivialInstance::staticMethod() { return {}; }

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Handle trivial_abi attribute for Microsoft ABI. (PR #88857)

2024-04-16 Thread Tobias Hieta via cfe-commits 


@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -std=c++11 -fcxx-exceptions 
-fexceptions -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: %[[STRUCT_TRIVIAL:.*]] = type { ptr }
+struct __attribute__((trivial_abi)) Trivial {
+  int *p;
+  Trivial() : p(0) {}
+  Trivial(const Trivial &) noexcept = default;
+};
+
+// CHECK-LABEL: define{{.*}} i64 @"?retTrivial@@YA?AUTrivial@@XZ"(
+// CHECK: %retval = alloca %[[STRUCT_TRIVIAL]], align 8
+// CHECK: %call = call noundef ptr @"??0Trivial@@QEAA@XZ"(ptr noundef nonnull 
align 8 dereferenceable(8) %retval)
+// CHECK: %coerce.dive = getelementptr inbounds %[[STRUCT_TRIVIAL]], ptr 
%retval, i32 0, i32 0
+// CHECK: %0 = load ptr, ptr %coerce.dive, align 8
+// CHECK: %coerce.val.pi = ptrtoint ptr %0 to i64
+// CHECK: ret i64 %coerce.val.pi
+Trivial retTrivial() {
+  Trivial s;
+  return s;
+}
+

tru wrote:

I checked your example there and the instanceMethod is no problem, but I 
noticed that the staticMethod doesn't contain a `sret` return value with this 
patch. Since we are not passing `this` to it - I think this is expected, but 
just want to make sure.

```
; Function Attrs: mustprogress noinline optnone uwtable
define dso_local i64 @"?staticMethod@TrivialInstance@@SA?AUTrivial@@XZ"() #0 
align 2 {
entry:
  %retval = alloca %struct.Trivial, align 8
  %call = call noundef ptr @"??0Trivial@@QEAA@XZ"(ptr noundef nonnull align 8 
dereferenceable(8) %retval)
  %coerce.dive = getelementptr inbounds %struct.Trivial, ptr %retval, i32 0, 
i32 0
  %0 = load ptr, ptr %coerce.dive, align 8
  %coerce.val.pi = ptrtoint ptr %0 to i64
  ret i64 %coerce.val.pi
}
```

https://github.com/llvm/llvm-project/pull/88857
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Handle trivial_abi attribute for Microsoft ABI. (PR #88857)

2024-04-16 Thread Reid Kleckner via cfe-commits 


@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -std=c++11 -fcxx-exceptions 
-fexceptions -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: %[[STRUCT_TRIVIAL:.*]] = type { ptr }
+struct __attribute__((trivial_abi)) Trivial {
+  int *p;
+  Trivial() : p(0) {}
+  Trivial(const Trivial &) noexcept = default;
+};
+
+// CHECK-LABEL: define{{.*}} i64 @"?retTrivial@@YA?AUTrivial@@XZ"(
+// CHECK: %retval = alloca %[[STRUCT_TRIVIAL]], align 8
+// CHECK: %call = call noundef ptr @"??0Trivial@@QEAA@XZ"(ptr noundef nonnull 
align 8 dereferenceable(8) %retval)
+// CHECK: %coerce.dive = getelementptr inbounds %[[STRUCT_TRIVIAL]], ptr 
%retval, i32 0, i32 0
+// CHECK: %0 = load ptr, ptr %coerce.dive, align 8
+// CHECK: %coerce.val.pi = ptrtoint ptr %0 to i64
+// CHECK: ret i64 %coerce.val.pi
+Trivial retTrivial() {
+  Trivial s;
+  return s;
+}
+

rnk wrote:

For completeness, please add the instance and static method return cases, since 
those are indirect for other reasons. For me, it's sufficient to check the 
function prototype, both of which should have an sret pointer:
https://godbolt.org/z/fn7vrGrKh

https://github.com/llvm/llvm-project/pull/88857
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Handle trivial_abi attribute for Microsoft ABI. (PR #88857)

2024-04-16 Thread Reid Kleckner via cfe-commits 


@@ -63,6 +63,10 @@ ABI Changes in This Version
   MSVC uses a different mangling for these objects, compatibility is not 
affected.
   (#GH85423).
 
+- The attribute ``trivial_abi`` now works when targetting the Microsoft ABI. 
Marking

rnk wrote:

It's overly broad to say it "works", and I suggest narrowing the scope of the 
release note. For example, there is a [major known 
issue](https://github.com/llvm/llvm-project/issues/69394) with 
`__is_trivially_relocatable`, which I think means that `trivial_abi` won't work 
the way people want it to in containers. Maybe try:
> Records carrying the ``trivial_abi`` attribute are now returned directly in 
> registers in more cases when using the Microsoft ABI. It is not possible to 
> pass ``trivial_abi`` records between MSVC and Clang, so there is no ABI 
> compatibility requirement. This is an ABI break with old versions of Clang. 
> (#GH87993) 

https://github.com/llvm/llvm-project/pull/88857
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Handle trivial_abi attribute for Microsoft ABI. (PR #88857)

2024-04-16 Thread via cfe-commits 

llvmbot wrote:




@llvm/pr-subscribers-clang-codegen

Author: Tobias Hieta (tru)


Changes

Previously the trivial_abi was ignored for records when targetting the 
microsoft abi, the MSVC rules where always enforced to ensure compatibility 
with MSVC. This commit changes it to be closer to the itanium abi when a record 
is marked with the trivial_abi attribute.

Fixes #87993

---
Full diff: https://github.com/llvm/llvm-project/pull/88857.diff


3 Files Affected:

- (modified) clang/docs/ReleaseNotes.rst (+4) 
- (modified) clang/lib/CodeGen/MicrosoftCXXABI.cpp (+5) 
- (added) clang/test/CodeGenCXX/trivial_abi_msvc.cpp (+21) 


``diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 76701dc723b6c3..c5241403711726 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -63,6 +63,10 @@ ABI Changes in This Version
   MSVC uses a different mangling for these objects, compatibility is not 
affected.
   (#GH85423).
 
+- The attribute ``trivial_abi`` now works when targetting the Microsoft ABI. 
Marking
+  a struct with this attribute will now cause clang to emit ABI similar to the 
Itanium
+  ABI. This is not compatible with the Microsoft ABI. (#GH87993).
+
 AST Dumping Potentially Breaking Changes
 
 
diff --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp 
b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
index d38a26940a3cb6..b930913badcd3f 100644
--- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -1105,6 +1105,11 @@ bool MicrosoftCXXABI::hasMostDerivedReturn(GlobalDecl 
GD) const {
 
 static bool isTrivialForMSVC(const CXXRecordDecl *RD, QualType Ty,
  CodeGenModule ) {
+  // If the record is marked with the trivial_abi attribute, we don't
+  // have to conform to the standard MSVC ABI.
+  if (RD->hasAttr())
+return true;
+
   // On AArch64, HVAs that can be passed in registers can also be returned
   // in registers. (Note this is using the MSVC definition of an HVA; see
   // isPermittedToBeHomogeneousAggregate().)
diff --git a/clang/test/CodeGenCXX/trivial_abi_msvc.cpp 
b/clang/test/CodeGenCXX/trivial_abi_msvc.cpp
new file mode 100644
index 00..8826a8224c6671
--- /dev/null
+++ b/clang/test/CodeGenCXX/trivial_abi_msvc.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -std=c++11 -fcxx-exceptions 
-fexceptions -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: %[[STRUCT_TRIVIAL:.*]] = type { ptr }
+struct __attribute__((trivial_abi)) Trivial {
+  int *p;
+  Trivial() : p(0) {}
+  Trivial(const Trivial &) noexcept = default;
+};
+
+// CHECK-LABEL: define{{.*}} i64 @"?retTrivial@@YA?AUTrivial@@XZ"(
+// CHECK: %retval = alloca %[[STRUCT_TRIVIAL]], align 8
+// CHECK: %call = call noundef ptr @"??0Trivial@@QEAA@XZ"(ptr noundef nonnull 
align 8 dereferenceable(8) %retval)
+// CHECK: %coerce.dive = getelementptr inbounds %[[STRUCT_TRIVIAL]], ptr 
%retval, i32 0, i32 0
+// CHECK: %0 = load ptr, ptr %coerce.dive, align 8
+// CHECK: %coerce.val.pi = ptrtoint ptr %0 to i64
+// CHECK: ret i64 %coerce.val.pi
+Trivial retTrivial() {
+  Trivial s;
+  return s;
+}
+

``




https://github.com/llvm/llvm-project/pull/88857
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Handle trivial_abi attribute for Microsoft ABI. (PR #88857)

2024-04-16 Thread Tobias Hieta via cfe-commits 

https://github.com/tru created https://github.com/llvm/llvm-project/pull/88857

Previously the trivial_abi was ignored for records when targetting the 
microsoft abi, the MSVC rules where always enforced to ensure compatibility 
with MSVC. This commit changes it to be closer to the itanium abi when a record 
is marked with the trivial_abi attribute.

Fixes #87993

>From 22f627cac7b2ea08a5569bb25d9c1d054b3a7cae Mon Sep 17 00:00:00 2001
From: Tobias Hieta 
Date: Tue, 16 Apr 2024 09:38:53 +0200
Subject: [PATCH] [clang] Handle trivial_abi attribute for Microsoft ABI.

Previously the trivial_abi was ignored for records when targetting
the microsoft abi, the MSVC rules where always enforced to ensure
compatibility with MSVC. This commit changes it to be closer to
the itanium abi when a record is marked with the trivial_abi attribute.

Fixes #87993
---
 clang/docs/ReleaseNotes.rst|  4 
 clang/lib/CodeGen/MicrosoftCXXABI.cpp  |  5 +
 clang/test/CodeGenCXX/trivial_abi_msvc.cpp | 21 +
 3 files changed, 30 insertions(+)
 create mode 100644 clang/test/CodeGenCXX/trivial_abi_msvc.cpp

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 76701dc723b6c3..c5241403711726 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -63,6 +63,10 @@ ABI Changes in This Version
   MSVC uses a different mangling for these objects, compatibility is not 
affected.
   (#GH85423).
 
+- The attribute ``trivial_abi`` now works when targetting the Microsoft ABI. 
Marking
+  a struct with this attribute will now cause clang to emit ABI similar to the 
Itanium
+  ABI. This is not compatible with the Microsoft ABI. (#GH87993).
+
 AST Dumping Potentially Breaking Changes
 
 
diff --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp 
b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
index d38a26940a3cb6..b930913badcd3f 100644
--- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -1105,6 +1105,11 @@ bool MicrosoftCXXABI::hasMostDerivedReturn(GlobalDecl 
GD) const {
 
 static bool isTrivialForMSVC(const CXXRecordDecl *RD, QualType Ty,
  CodeGenModule ) {
+  // If the record is marked with the trivial_abi attribute, we don't
+  // have to conform to the standard MSVC ABI.
+  if (RD->hasAttr())
+return true;
+
   // On AArch64, HVAs that can be passed in registers can also be returned
   // in registers. (Note this is using the MSVC definition of an HVA; see
   // isPermittedToBeHomogeneousAggregate().)
diff --git a/clang/test/CodeGenCXX/trivial_abi_msvc.cpp 
b/clang/test/CodeGenCXX/trivial_abi_msvc.cpp
new file mode 100644
index 00..8826a8224c6671
--- /dev/null
+++ b/clang/test/CodeGenCXX/trivial_abi_msvc.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -std=c++11 -fcxx-exceptions 
-fexceptions -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: %[[STRUCT_TRIVIAL:.*]] = type { ptr }
+struct __attribute__((trivial_abi)) Trivial {
+  int *p;
+  Trivial() : p(0) {}
+  Trivial(const Trivial &) noexcept = default;
+};
+
+// CHECK-LABEL: define{{.*}} i64 @"?retTrivial@@YA?AUTrivial@@XZ"(
+// CHECK: %retval = alloca %[[STRUCT_TRIVIAL]], align 8
+// CHECK: %call = call noundef ptr @"??0Trivial@@QEAA@XZ"(ptr noundef nonnull 
align 8 dereferenceable(8) %retval)
+// CHECK: %coerce.dive = getelementptr inbounds %[[STRUCT_TRIVIAL]], ptr 
%retval, i32 0, i32 0
+// CHECK: %0 = load ptr, ptr %coerce.dive, align 8
+// CHECK: %coerce.val.pi = ptrtoint ptr %0 to i64
+// CHECK: ret i64 %coerce.val.pi
+Trivial retTrivial() {
+  Trivial s;
+  return s;
+}
+

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits