[clang] [LoongArch] Fix ABI mismatch with gcc/g++ about empty structs passing (PR #70320)

2023-10-26 Thread Lu Weining via cfe-commits 

https://github.com/SixWeining created 
https://github.com/llvm/llvm-project/pull/70320

How empty structs (not as fields of container struct) are passed in C++ is not 
explicitly documented in psABI. However, this patch fixes the mismatch with g++.

Note that the unnamed bitfield case `struct { int : 1; }` in C is also fixed. 
Previously clang regards it as an empty struct and then ignores it when 
passing. Now size of the struct is counted; since it's size is not 0, clang 
will not ignore it even in C.

While https://reviews.llvm.org/D156116 fixed the handling of empty struct when 
considering eligibility of the container struct for the FP calling convention 
('flattening'), this patch fixes the handling of passing the empty struct 
itself.

Fix https://github.com/llvm/llvm-project/issues/70319

>From 3ed0605126e2cfd94e58dd38c2f58764e32e2057 Mon Sep 17 00:00:00 2001
From: Weining Lu 
Date: Thu, 26 Oct 2023 13:40:27 +0800
Subject: [PATCH] [LoongArch] Fix ABI mismatch with gcc/g++ about empty structs
 passing

How empty structs (not as fields of container struct) are passed in C++
is not explicitly documented in psABI. However, this patch fixes the
mismatch with g++.

Note that the unnamed bitfield case `struct { int : 1; }` in C is also
fixed. Previously clang regards it as an empty struct and then ignores
it when passing. Now size of the struct is counted; since it's size is
not 0, clang will not ignore it even in C.

While https://reviews.llvm.org/D156116 fixed the handling of empty
struct when considering eligibility of the container struct for the FP
calling convention ('flattening'), this patch fixes the handling of
passing the empty struct itself.

Fix https://github.com/llvm/llvm-project/issues/70319
---
 clang/lib/CodeGen/Targets/LoongArch.cpp| 10 ++
 clang/test/CodeGen/LoongArch/abi-lp64d-empty-structs.c |  8 
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/clang/lib/CodeGen/Targets/LoongArch.cpp 
b/clang/lib/CodeGen/Targets/LoongArch.cpp
index 7483bf6d6d1e8e2..bc508a99da9cedf 100644
--- a/clang/lib/CodeGen/Targets/LoongArch.cpp
+++ b/clang/lib/CodeGen/Targets/LoongArch.cpp
@@ -308,12 +308,14 @@ ABIArgInfo 
LoongArchABIInfo::classifyArgumentType(QualType Ty, bool IsFixed,
CGCXXABI::RAA_DirectInMemory);
   }
 
-  // Ignore empty structs/unions.
-  if (isEmptyRecord(getContext(), Ty, true))
-return ABIArgInfo::getIgnore();
-
   uint64_t Size = getContext().getTypeSize(Ty);
 
+  // Ignore empty struct or union whose size is zero, e.g. `struct { }` in C or
+  // `struct { int a[0]; }` in C++. In C++, `struct { }` is empty but it's size
+  // is 1 byte and g++ doesn't ignore it; clang++ matches this behaviour.
+  if (isEmptyRecord(getContext(), Ty, true) && Size == 0)
+return ABIArgInfo::getIgnore();
+
   // Pass floating point values via FARs if possible.
   if (IsFixed && Ty->isFloatingType() && !Ty->isComplexType() &&
   FRLen >= Size && FARsLeft) {
diff --git a/clang/test/CodeGen/LoongArch/abi-lp64d-empty-structs.c 
b/clang/test/CodeGen/LoongArch/abi-lp64d-empty-structs.c
index d0daafac336ec0c..281b7b15841a999 100644
--- a/clang/test/CodeGen/LoongArch/abi-lp64d-empty-structs.c
+++ b/clang/test/CodeGen/LoongArch/abi-lp64d-empty-structs.c
@@ -93,7 +93,7 @@ struct s9 test_s9(struct s9 a) {
 }
 
 // CHECK-C: define{{.*}} void @test_s10()
-// CHECK-CXX: define{{.*}} void @_Z8test_s103s10()
+// CHECK-CXX: define{{.*}} i64 @_Z8test_s103s10(i64 {{.*}})
 struct s10 { };
 struct s10 test_s10(struct s10 a) {
   return a;
@@ -128,14 +128,14 @@ struct s14 test_s14(struct s14 a) {
 }
 
 // CHECK-C: define{{.*}} void @test_s15()
-// CHECK-CXX: define{{.*}} void @_Z8test_s153s15()
+// CHECK-CXX: define{{.*}} i64 @_Z8test_s153s15(i64 {{.*}})
 struct s15 { int : 0; };
 struct s15 test_s15(struct s15 a) {
   return a;
 }
 
-// CHECK-C: define{{.*}} void @test_s16()
-// CHECK-CXX: define{{.*}} void @_Z8test_s163s16()
+// CHECK-C: define{{.*}} i64 @test_s16(i64 {{.*}})
+// CHECK-CXX: define{{.*}} i64 @_Z8test_s163s16(i64 {{.*}})
 struct s16 { int : 1; };
 struct s16 test_s16(struct s16 a) {
   return a;

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

[clang] [LoongArch] Fix ABI mismatch with gcc/g++ about empty structs passing (PR #70320)

2023-10-26 Thread via cfe-commits 

llvmbot wrote:




@llvm/pr-subscribers-clang-codegen

Author: Lu Weining (SixWeining)


Changes

How empty structs (not as fields of container struct) are passed in C++ is not 
explicitly documented in psABI. However, this patch fixes the mismatch with g++.

Note that the unnamed bitfield case `struct { int : 1; }` in C is also fixed. 
Previously clang regards it as an empty struct and then ignores it when 
passing. Now size of the struct is counted; since it's size is not 0, clang 
will not ignore it even in C.

While https://reviews.llvm.org/D156116 fixed the handling of empty struct when 
considering eligibility of the container struct for the FP calling convention 
('flattening'), this patch fixes the handling of passing the empty struct 
itself.

Fix https://github.com/llvm/llvm-project/issues/70319

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


2 Files Affected:

- (modified) clang/lib/CodeGen/Targets/LoongArch.cpp (+6-4) 
- (modified) clang/test/CodeGen/LoongArch/abi-lp64d-empty-structs.c (+4-4) 


``diff
diff --git a/clang/lib/CodeGen/Targets/LoongArch.cpp 
b/clang/lib/CodeGen/Targets/LoongArch.cpp
index 7483bf6d6d1e8e2..bc508a99da9cedf 100644
--- a/clang/lib/CodeGen/Targets/LoongArch.cpp
+++ b/clang/lib/CodeGen/Targets/LoongArch.cpp
@@ -308,12 +308,14 @@ ABIArgInfo 
LoongArchABIInfo::classifyArgumentType(QualType Ty, bool IsFixed,
CGCXXABI::RAA_DirectInMemory);
   }
 
-  // Ignore empty structs/unions.
-  if (isEmptyRecord(getContext(), Ty, true))
-return ABIArgInfo::getIgnore();
-
   uint64_t Size = getContext().getTypeSize(Ty);
 
+  // Ignore empty struct or union whose size is zero, e.g. `struct { }` in C or
+  // `struct { int a[0]; }` in C++. In C++, `struct { }` is empty but it's size
+  // is 1 byte and g++ doesn't ignore it; clang++ matches this behaviour.
+  if (isEmptyRecord(getContext(), Ty, true) && Size == 0)
+return ABIArgInfo::getIgnore();
+
   // Pass floating point values via FARs if possible.
   if (IsFixed && Ty->isFloatingType() && !Ty->isComplexType() &&
   FRLen >= Size && FARsLeft) {
diff --git a/clang/test/CodeGen/LoongArch/abi-lp64d-empty-structs.c 
b/clang/test/CodeGen/LoongArch/abi-lp64d-empty-structs.c
index d0daafac336ec0c..281b7b15841a999 100644
--- a/clang/test/CodeGen/LoongArch/abi-lp64d-empty-structs.c
+++ b/clang/test/CodeGen/LoongArch/abi-lp64d-empty-structs.c
@@ -93,7 +93,7 @@ struct s9 test_s9(struct s9 a) {
 }
 
 // CHECK-C: define{{.*}} void @test_s10()
-// CHECK-CXX: define{{.*}} void @_Z8test_s103s10()
+// CHECK-CXX: define{{.*}} i64 @_Z8test_s103s10(i64 {{.*}})
 struct s10 { };
 struct s10 test_s10(struct s10 a) {
   return a;
@@ -128,14 +128,14 @@ struct s14 test_s14(struct s14 a) {
 }
 
 // CHECK-C: define{{.*}} void @test_s15()
-// CHECK-CXX: define{{.*}} void @_Z8test_s153s15()
+// CHECK-CXX: define{{.*}} i64 @_Z8test_s153s15(i64 {{.*}})
 struct s15 { int : 0; };
 struct s15 test_s15(struct s15 a) {
   return a;
 }
 
-// CHECK-C: define{{.*}} void @test_s16()
-// CHECK-CXX: define{{.*}} void @_Z8test_s163s16()
+// CHECK-C: define{{.*}} i64 @test_s16(i64 {{.*}})
+// CHECK-CXX: define{{.*}} i64 @_Z8test_s163s16(i64 {{.*}})
 struct s16 { int : 1; };
 struct s16 test_s16(struct s16 a) {
   return a;

``




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

[clang] [LoongArch] Fix ABI mismatch with gcc/g++ about empty structs passing (PR #70320)

2023-10-26 Thread Lu Weining via cfe-commits 

SixWeining wrote:

Hi @xen0n @xry111, what do you think about this change? If it is ok, I'd like 
to checrry-pick to release/17.x (hope it's not too late...).

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

[clang] [LoongArch] Fix ABI mismatch with gcc/g++ about empty structs passing (PR #70320)

2023-10-30 Thread via cfe-commits 

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


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

[clang] [LoongArch] Fix ABI mismatch with gcc/g++ about empty structs passing (PR #70320)

2023-10-30 Thread Xi Ruoyao via cfe-commits 

xry111 wrote:

LGTM.  I've no permission to make a formal ("GitHub style") approval.

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

[clang] [LoongArch] Fix ABI mismatch with gcc/g++ about empty structs passing (PR #70320)

2023-10-31 Thread Lu Weining via cfe-commits 

SixWeining wrote:

> LGTM. I've no permission to make a formal ("GitHub style") approval.

Thanks. You can request to join the llvm organization via 
https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access firstly. 
Then you can make formal ("Github style") approval.

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

[clang] [LoongArch] Fix ABI mismatch with gcc/g++ about empty structs passing (PR #70320)

2023-10-31 Thread Lu Weining via cfe-commits 

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