[PATCH] D74455: [MS] Pass aligned, non-trivially copyable things indirectly on x86

[Reid Kleckner via Phabricator via cfe-commits]

rnk abandoned this revision.
rnk added a comment.

I actually forgot about this change entirely and uploaded and landed a new one:
https://reviews.llvm.org/D87923

I suppose they are functionally different: if we have an overaligned type that 
can be passed in registers due to [[trivial_abi]], we should do that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74455

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

[PATCH] D74455: [MS] Pass aligned, non-trivially copyable things indirectly on x86

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74455



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

[PATCH] D74455: [MS] Pass aligned, non-trivially copyable things indirectly on x86

[Reid Kleckner via Phabricator via cfe-commits]

rnk updated this revision to Diff 245491.
rnk added a comment.

- rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74455

Files:
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/test/CodeGenCXX/inalloca-overaligned.cpp


Index: clang/test/CodeGenCXX/inalloca-overaligned.cpp
===
--- clang/test/CodeGenCXX/inalloca-overaligned.cpp
+++ clang/test/CodeGenCXX/inalloca-overaligned.cpp
@@ -4,10 +4,6 @@
 // MSVC passes overaligned types indirectly since MSVC 2015. Make sure that
 // works with inalloca.
 
-// FIXME: Pass non-trivial *and* overaligned types indirectly. Right now the 
C++
-// ABI rules say to use inalloca, and they take precedence, so it's not easy to
-// implement this.
-
 
 struct NonTrivial {
   NonTrivial();
@@ -50,3 +46,27 @@
 // CHECK: getelementptr inbounds <{ %struct.NonTrivial, %struct.OverAligned* 
}>, <{ %struct.NonTrivial, %struct.OverAligned* }>* %{{.*}}, i32 0, i32 1
 // CHECK: store %struct.OverAligned* [[TMP]], %struct.OverAligned** %{{.*}}, 
align 4
 // CHECK: call i32 @"?receive_inalloca_overaligned@@Y{{.*}}"(<{ 
%struct.NonTrivial, %struct.OverAligned* }>* inalloca %argmem)
+
+struct __declspec(align(64)) Both {
+  Both();
+  Both(const Both );
+  int x;
+};
+
+int receive_both1(Both b) {
+  return b.x;
+}
+// CHECK-LABEL: define dso_local i32 @"?receive_both1@@Y{{.*}}"
+// CHECK-SAME: (%struct.Both* %b)
+
+int receive_both2(Both x, Both y) {
+  return x.x + y.x;
+}
+// CHECK-LABEL: define dso_local i32 @"?receive_both2@@Y{{.*}}"
+// CHECK-SAME: (%struct.Both* %x, %struct.Both* %y)
+
+int receive_nt_and_both(NonTrivial x, Both y) {
+  return x.x + y.x;
+}
+// CHECK-LABEL: define dso_local i32 @"?receive_nt_and_both@@Y{{.*}}"
+// CHECK-SAME: (<{ %struct.NonTrivial, %struct.Both* }>* inalloca %0)
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -821,18 +821,19 @@
 return !RD->canPassInRegisters() ? RAA_Indirect : RAA_Default;
 
   case llvm::Triple::x86:
-// All record arguments are passed in memory on x86.  Decide whether to
-// construct the object directly in argument memory, or to construct the
-// argument elsewhere and copy the bytes during the call.
-
-// If C++ prohibits us from making a copy, construct the arguments directly
-// into argument memory.
-if (!RD->canPassInRegisters())
-  return RAA_DirectInMemory;
-
-// Otherwise, construct the argument into a temporary and copy the bytes
-// into the outgoing argument memory.
-return RAA_Default;
+// If C++ allows us to copy the struct bytes, use the normal C convention
+// rules.
+if (RD->canPassInRegisters())
+  return RAA_Default;
+
+// If the type has a required alignment (alignas, __declspec(align)), MSVC
+// passes it indirectly.
+if (getContext().isAlignmentRequired(getContext().getTypeDeclType(RD)))
+  return RAA_Indirect;
+
+// In the worst case, the argument must be constructed directly into the
+// outgoing argument memory slots.
+return RAA_DirectInMemory;
 
   case llvm::Triple::x86_64:
   case llvm::Triple::aarch64:


Index: clang/test/CodeGenCXX/inalloca-overaligned.cpp
===
--- clang/test/CodeGenCXX/inalloca-overaligned.cpp
+++ clang/test/CodeGenCXX/inalloca-overaligned.cpp
@@ -4,10 +4,6 @@
 // MSVC passes overaligned types indirectly since MSVC 2015. Make sure that
 // works with inalloca.
 
-// FIXME: Pass non-trivial *and* overaligned types indirectly. Right now the C++
-// ABI rules say to use inalloca, and they take precedence, so it's not easy to
-// implement this.
-
 
 struct NonTrivial {
   NonTrivial();
@@ -50,3 +46,27 @@
 // CHECK: getelementptr inbounds <{ %struct.NonTrivial, %struct.OverAligned* }>, <{ %struct.NonTrivial, %struct.OverAligned* }>* %{{.*}}, i32 0, i32 1
 // CHECK: store %struct.OverAligned* [[TMP]], %struct.OverAligned** %{{.*}}, align 4
 // CHECK: call i32 @"?receive_inalloca_overaligned@@Y{{.*}}"(<{ %struct.NonTrivial, %struct.OverAligned* }>* inalloca %argmem)
+
+struct __declspec(align(64)) Both {
+  Both();
+  Both(const Both );
+  int x;
+};
+
+int receive_both1(Both b) {
+  return b.x;
+}
+// CHECK-LABEL: define dso_local i32 @"?receive_both1@@Y{{.*}}"
+// CHECK-SAME: (%struct.Both* %b)
+
+int receive_both2(Both x, Both y) {
+  return x.x + y.x;
+}
+// CHECK-LABEL: define dso_local i32 @"?receive_both2@@Y{{.*}}"
+// CHECK-SAME: (%struct.Both* %x, %struct.Both* %y)
+
+int receive_nt_and_both(NonTrivial x, Both y) {
+  return x.x + y.x;
+}
+// CHECK-LABEL: define dso_local i32 @"?receive_nt_and_both@@Y{{.*}}"
+// CHECK-SAME: (<{ %struct.NonTrivial, %struct.Both* }>* inalloca %0)
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp

[PATCH] D74455: [MS] Pass aligned, non-trivially copyable things indirectly on x86

[Reid Kleckner via Phabricator via cfe-commits]

rnk created this revision.
rnk added reviewers: craig.topper, erichkeane.
Herald added a project: clang.

This is a follow-up to case 1 in D72114 .

The C++ argument classification logic supersedes the C rules, so we have
to add a second check for overaligned records in the C++ argument
classification logic. Otherwise, clang will pass such records using
inalloca.

This has the interesting consequence of making it possible to opt a
record out of inalloca by adding a trivial alignment requirement
(alignas(void*, uint64_t)) to the record.

Fixes the last known corner case to PR44395


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74455

Files:
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/test/CodeGenCXX/inalloca-overaligned.cpp


Index: clang/test/CodeGenCXX/inalloca-overaligned.cpp
===
--- clang/test/CodeGenCXX/inalloca-overaligned.cpp
+++ clang/test/CodeGenCXX/inalloca-overaligned.cpp
@@ -4,10 +4,6 @@
 // MSVC passes overaligned types indirectly since MSVC 2015. Make sure that
 // works with inalloca.
 
-// FIXME: Pass non-trivial *and* overaligned types indirectly. Right now the 
C++
-// ABI rules say to use inalloca, and they take precedence, so it's not easy to
-// implement this.
-
 
 struct NonTrivial {
   NonTrivial();
@@ -50,3 +46,27 @@
 // CHECK: getelementptr inbounds <{ %struct.NonTrivial, %struct.OverAligned* 
}>, <{ %struct.NonTrivial, %struct.OverAligned* }>* %{{.*}}, i32 0, i32 1
 // CHECK: store %struct.OverAligned* [[TMP]], %struct.OverAligned** %{{.*}}, 
align 4
 // CHECK: call i32 @"?receive_inalloca_overaligned@@Y{{.*}}"(<{ 
%struct.NonTrivial, %struct.OverAligned* }>* inalloca %argmem)
+
+struct __declspec(align(64)) Both {
+  Both();
+  Both(const Both );
+  int x;
+};
+
+int receive_both1(Both b) {
+  return b.x;
+}
+// CHECK-LABEL: define dso_local i32 @"?receive_both1@@Y{{.*}}"
+// CHECK-SAME: (%struct.Both* %b)
+
+int receive_both2(Both x, Both y) {
+  return x.x + y.x;
+}
+// CHECK-LABEL: define dso_local i32 @"?receive_both2@@Y{{.*}}"
+// CHECK-SAME: (%struct.Both* %x, %struct.Both* %y)
+
+int receive_nt_and_both(NonTrivial x, Both y) {
+  return x.x + y.x;
+}
+// CHECK-LABEL: define dso_local i32 @"?receive_nt_and_both@@Y{{.*}}"
+// CHECK-SAME: (<{ %struct.NonTrivial, %struct.Both* }>* inalloca %0)
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -821,18 +821,19 @@
 return !RD->canPassInRegisters() ? RAA_Indirect : RAA_Default;
 
   case llvm::Triple::x86:
-// All record arguments are passed in memory on x86.  Decide whether to
-// construct the object directly in argument memory, or to construct the
-// argument elsewhere and copy the bytes during the call.
-
-// If C++ prohibits us from making a copy, construct the arguments directly
-// into argument memory.
-if (!RD->canPassInRegisters())
-  return RAA_DirectInMemory;
-
-// Otherwise, construct the argument into a temporary and copy the bytes
-// into the outgoing argument memory.
-return RAA_Default;
+// If C++ allows us to copy the struct bytes, use the normal C convention
+// rules.
+if (RD->canPassInRegisters())
+  return RAA_Default;
+
+// If the type has a required alignment (alignas, __declspec(align)), MSVC
+// passes it indirectly.
+if (getContext().isAlignmentRequired(getContext().getTypeDeclType(RD)))
+  return RAA_Indirect;
+
+// In the worst case, the argument must be constructed directly into the
+// outgoing argument memory slots.
+return RAA_DirectInMemory;
 
   case llvm::Triple::x86_64:
   case llvm::Triple::aarch64:


Index: clang/test/CodeGenCXX/inalloca-overaligned.cpp
===
--- clang/test/CodeGenCXX/inalloca-overaligned.cpp
+++ clang/test/CodeGenCXX/inalloca-overaligned.cpp
@@ -4,10 +4,6 @@
 // MSVC passes overaligned types indirectly since MSVC 2015. Make sure that
 // works with inalloca.
 
-// FIXME: Pass non-trivial *and* overaligned types indirectly. Right now the C++
-// ABI rules say to use inalloca, and they take precedence, so it's not easy to
-// implement this.
-
 
 struct NonTrivial {
   NonTrivial();
@@ -50,3 +46,27 @@
 // CHECK: getelementptr inbounds <{ %struct.NonTrivial, %struct.OverAligned* }>, <{ %struct.NonTrivial, %struct.OverAligned* }>* %{{.*}}, i32 0, i32 1
 // CHECK: store %struct.OverAligned* [[TMP]], %struct.OverAligned** %{{.*}}, align 4
 // CHECK: call i32 @"?receive_inalloca_overaligned@@Y{{.*}}"(<{ %struct.NonTrivial, %struct.OverAligned* }>* inalloca %argmem)
+
+struct __declspec(align(64)) Both {
+  Both();
+  Both(const Both );
+  int x;
+};
+
+int receive_both1(Both b) {
+  return b.x;
+}
+// CHECK-LABEL: define dso_local i32 @"?receive_both1@@Y{{.*}}"
+//