[clang] 0fa004e - Revert "[clang][RISCV] Fix ABI handling of empty structs with hard FP calling conventions in C++"
Alex Bradbury via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 24 09:00:12 PDT 2023
Author: Alex Bradbury
Date: 2023-07-24T16:58:48+01:00
New Revision: 0fa004e0724b4398f999b8bffbc37f524e41f66e
URL: https://github.com/llvm/llvm-project/commit/0fa004e0724b4398f999b8bffbc37f524e41f66e
DIFF: https://github.com/llvm/llvm-project/commit/0fa004e0724b4398f999b8bffbc37f524e41f66e.diff
LOG: Revert "[clang][RISCV] Fix ABI handling of empty structs with hard FP calling conventions in C++"
This reverts commit 17a58b3ca7ec18585e9ea8ed8b39d72fe36fb6cb and the
minor documentation fix 569e99a471f618b7fdf045d5e96f21d3e3a7f898.
An issue was reported in https://reviews.llvm.org/D142327#inline-1510301
so reverting until it can be investigated and fixed.
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/lib/CodeGen/ABIInfoImpl.cpp
clang/lib/CodeGen/ABIInfoImpl.h
clang/lib/CodeGen/Targets/RISCV.cpp
clang/test/CodeGen/RISCV/abi-empty-structs.c
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 4e60efa1c025ed..520e57532467db 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -906,9 +906,6 @@ RISC-V Support
- The rules for ordering of extensions in ``-march`` strings were relaxed. A
canonical ordering is no longer enforced on ``z*``, ``s*``, and ``x*``
prefixed extensions.
-- An ABI mismatch between GCC and Clang related to the handling of empty
- structs in C++ parameter passing under the hard floating point calling
- conventions was fixed.
CUDA/HIP Language Changes
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/CodeGen/ABIInfoImpl.cpp b/clang/lib/CodeGen/ABIInfoImpl.cpp
index 2b20d5a13346d3..7c30cecfdb9b77 100644
--- a/clang/lib/CodeGen/ABIInfoImpl.cpp
+++ b/clang/lib/CodeGen/ABIInfoImpl.cpp
@@ -246,7 +246,7 @@ Address CodeGen::emitMergePHI(CodeGenFunction &CGF, Address Addr1,
}
bool CodeGen::isEmptyField(ASTContext &Context, const FieldDecl *FD,
- bool AllowArrays, bool AsIfNoUniqueAddr) {
+ bool AllowArrays) {
if (FD->isUnnamedBitfield())
return true;
@@ -280,14 +280,13 @@ bool CodeGen::isEmptyField(ASTContext &Context, const FieldDecl *FD,
// not arrays of records, so we must also check whether we stripped off an
// array type above.
if (isa<CXXRecordDecl>(RT->getDecl()) &&
- (WasArray || (!AsIfNoUniqueAddr && !FD->hasAttr<NoUniqueAddressAttr>())))
+ (WasArray || !FD->hasAttr<NoUniqueAddressAttr>()))
return false;
- return isEmptyRecord(Context, FT, AllowArrays, AsIfNoUniqueAddr);
+ return isEmptyRecord(Context, FT, AllowArrays);
}
-bool CodeGen::isEmptyRecord(ASTContext &Context, QualType T, bool AllowArrays,
- bool AsIfNoUniqueAddr) {
+bool CodeGen::isEmptyRecord(ASTContext &Context, QualType T, bool AllowArrays) {
const RecordType *RT = T->getAs<RecordType>();
if (!RT)
return false;
@@ -298,11 +297,11 @@ bool CodeGen::isEmptyRecord(ASTContext &Context, QualType T, bool AllowArrays,
// If this is a C++ record, check the bases first.
if (const CXXRecordDecl *CXXRD = dyn_cast<CXXRecordDecl>(RD))
for (const auto &I : CXXRD->bases())
- if (!isEmptyRecord(Context, I.getType(), true, AsIfNoUniqueAddr))
+ if (!isEmptyRecord(Context, I.getType(), true))
return false;
for (const auto *I : RD->fields())
- if (!isEmptyField(Context, I, AllowArrays, AsIfNoUniqueAddr))
+ if (!isEmptyField(Context, I, AllowArrays))
return false;
return true;
}
diff --git a/clang/lib/CodeGen/ABIInfoImpl.h b/clang/lib/CodeGen/ABIInfoImpl.h
index afde08ba100cf0..5f0cc289af68b3 100644
--- a/clang/lib/CodeGen/ABIInfoImpl.h
+++ b/clang/lib/CodeGen/ABIInfoImpl.h
@@ -122,19 +122,13 @@ Address emitMergePHI(CodeGenFunction &CGF, Address Addr1,
llvm::BasicBlock *Block2, const llvm::Twine &Name = "");
/// isEmptyField - Return true iff a the field is "empty", that is it
-/// is an unnamed bit-field or an (array of) empty record(s). If
-/// AsIfNoUniqueAddr is true, then C++ record fields are considered empty if
-/// the [[no_unique_address]] attribute would have made them empty.
-bool isEmptyField(ASTContext &Context, const FieldDecl *FD, bool AllowArrays,
- bool AsIfNoUniqueAddr = false);
+/// is an unnamed bit-field or an (array of) empty record(s).
+bool isEmptyField(ASTContext &Context, const FieldDecl *FD, bool AllowArrays);
/// isEmptyRecord - Return true iff a structure contains only empty
/// fields. Note that a structure with a flexible array member is not
-/// considered empty. If AsIfNoUniqueAddr is true, then C++ record fields are
-/// considered empty if the [[no_unique_address]] attribute would have made
-/// them empty.
-bool isEmptyRecord(ASTContext &Context, QualType T, bool AllowArrays,
- bool AsIfNoUniqueAddr = false);
+/// considered empty.
+bool isEmptyRecord(ASTContext &Context, QualType T, bool AllowArrays);
/// isSingleElementStruct - Determine if a structure is a "single
/// element struct", i.e. it has exactly one non-empty field or
diff --git a/clang/lib/CodeGen/Targets/RISCV.cpp b/clang/lib/CodeGen/Targets/RISCV.cpp
index 4420ff0befaf30..b6d8ae462675c4 100644
--- a/clang/lib/CodeGen/Targets/RISCV.cpp
+++ b/clang/lib/CodeGen/Targets/RISCV.cpp
@@ -152,13 +152,6 @@ bool RISCVABIInfo::detectFPCCEligibleStructHelper(QualType Ty, CharUnits CurOff,
if (const ConstantArrayType *ATy = getContext().getAsConstantArrayType(Ty)) {
uint64_t ArraySize = ATy->getSize().getZExtValue();
QualType EltTy = ATy->getElementType();
- // Non-zero-length arrays of empty records make the struct ineligible for
- // the FP calling convention in C++.
- if (const auto *RTy = EltTy->getAs<RecordType>()) {
- if (ArraySize != 0 && isa<CXXRecordDecl>(RTy->getDecl()) &&
- isEmptyRecord(getContext(), EltTy, true, true))
- return false;
- }
CharUnits EltSize = getContext().getTypeSizeInChars(EltTy);
for (uint64_t i = 0; i < ArraySize; ++i) {
bool Ret = detectFPCCEligibleStructHelper(EltTy, CurOff, Field1Ty,
@@ -175,7 +168,7 @@ bool RISCVABIInfo::detectFPCCEligibleStructHelper(QualType Ty, CharUnits CurOff,
// copy constructor are not eligible for the FP calling convention.
if (getRecordArgABI(Ty, CGT.getCXXABI()))
return false;
- if (isEmptyRecord(getContext(), Ty, true, true))
+ if (isEmptyRecord(getContext(), Ty, true))
return true;
const RecordDecl *RD = RTy->getDecl();
// Unions aren't eligible unless they're empty (which is caught above).
diff --git a/clang/test/CodeGen/RISCV/abi-empty-structs.c b/clang/test/CodeGen/RISCV/abi-empty-structs.c
index 2a59fbae81ebcc..94cc29c95c293d 100644
--- a/clang/test/CodeGen/RISCV/abi-empty-structs.c
+++ b/clang/test/CodeGen/RISCV/abi-empty-structs.c
@@ -19,9 +19,8 @@
#include <stdint.h>
// Fields containing empty structs or unions are ignored when flattening
-// structs for the hard FP ABIs, even in C++. The rules for arrays of empty
-// structs or unions are subtle and documented in
-// <https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc#hardware-floating-point-calling-convention>.
+// structs for the hard FP ABIs, even in C++.
+// FIXME: This isn't currently respected.
struct empty { struct { struct { } e; }; };
struct s1 { struct empty e; float f; };
@@ -30,9 +29,13 @@ struct s1 { struct empty e; float f; };
// CHECK-C-SAME: (float [[TMP0:%.*]]) #[[ATTR0:[0-9]+]] {
// CHECK-C: entry:
//
-// CHECK-CXX-LABEL: define dso_local float @_Z7test_s12s1
-// CHECK-CXX-SAME: (float [[TMP0:%.*]]) #[[ATTR0:[0-9]+]] {
-// CHECK-CXX: entry:
+// CHECK32-CXX-LABEL: define dso_local [2 x i32] @_Z7test_s12s1
+// CHECK32-CXX-SAME: ([2 x i32] [[A_COERCE:%.*]]) #[[ATTR0:[0-9]+]] {
+// CHECK32-CXX: entry:
+//
+// CHECK64-CXX-LABEL: define dso_local i64 @_Z7test_s12s1
+// CHECK64-CXX-SAME: (i64 [[A_COERCE:%.*]]) #[[ATTR0:[0-9]+]] {
+// CHECK64-CXX: entry:
//
struct s1 test_s1(struct s1 a) {
return a;
@@ -44,9 +47,13 @@ struct s2 { struct empty e; int32_t i; float f; };
// CHECK-C-SAME: (i32 [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
// CHECK-C: entry:
//
-// CHECK-CXX-LABEL: define dso_local { i32, float } @_Z7test_s22s2
-// CHECK-CXX-SAME: (i32 [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
-// CHECK-CXX: entry:
+// CHECK32-CXX-LABEL: define dso_local void @_Z7test_s22s2
+// CHECK32-CXX-SAME: (ptr noalias sret([[STRUCT_S2:%.*]]) align 4 [[AGG_RESULT:%.*]], ptr noundef [[A:%.*]]) #[[ATTR0]] {
+// CHECK32-CXX: entry:
+//
+// CHECK64-CXX-LABEL: define dso_local [2 x i64] @_Z7test_s22s2
+// CHECK64-CXX-SAME: ([2 x i64] [[A_COERCE:%.*]]) #[[ATTR0]] {
+// CHECK64-CXX: entry:
//
struct s2 test_s2(struct s2 a) {
return a;
@@ -58,9 +65,13 @@ struct s3 { struct empty e; float f; float g; };
// CHECK-C-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
// CHECK-C: entry:
//
-// CHECK-CXX-LABEL: define dso_local { float, float } @_Z7test_s32s3
-// CHECK-CXX-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
-// CHECK-CXX: entry:
+// CHECK32-CXX-LABEL: define dso_local void @_Z7test_s32s3
+// CHECK32-CXX-SAME: (ptr noalias sret([[STRUCT_S3:%.*]]) align 4 [[AGG_RESULT:%.*]], ptr noundef [[A:%.*]]) #[[ATTR0]] {
+// CHECK32-CXX: entry:
+//
+// CHECK64-CXX-LABEL: define dso_local [2 x i64] @_Z7test_s32s3
+// CHECK64-CXX-SAME: ([2 x i64] [[A_COERCE:%.*]]) #[[ATTR0]] {
+// CHECK64-CXX: entry:
//
struct s3 test_s3(struct s3 a) {
return a;
@@ -72,9 +83,13 @@ struct s4 { struct empty e; float __complex__ c; };
// CHECK-C-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
// CHECK-C: entry:
//
-// CHECK-CXX-LABEL: define dso_local { float, float } @_Z7test_s42s4
-// CHECK-CXX-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] {
-// CHECK-CXX: entry:
+// CHECK32-CXX-LABEL: define dso_local void @_Z7test_s42s4
+// CHECK32-CXX-SAME: (ptr noalias sret([[STRUCT_S4:%.*]]) align 4 [[AGG_RESULT:%.*]], ptr noundef [[A:%.*]]) #[[ATTR0]] {
+// CHECK32-CXX: entry:
+//
+// CHECK64-CXX-LABEL: define dso_local [2 x i64] @_Z7test_s42s4
+// CHECK64-CXX-SAME: ([2 x i64] [[A_COERCE:%.*]]) #[[ATTR0]] {
+// CHECK64-CXX: entry:
//
struct s4 test_s4(struct s4 a) {
return a;
@@ -127,7 +142,7 @@ struct s7 { struct empty e[0]; float f; };
// CHECK-C: entry:
//
// CHECK-CXX-LABEL: define dso_local float @_Z7test_s72s7
-// CHECK-CXX-SAME: (float [[TMP0:%.*]]) #[[ATTR0]] {
+// CHECK-CXX-SAME: (float [[TMP0:%.*]]) #[[ATTR0:[0-9]+]] {
// CHECK-CXX: entry:
//
struct s7 test_s7(struct s7 a) {
@@ -141,9 +156,13 @@ struct s8 { struct empty_arr0 e; float f; };
// CHECK-C-SAME: (float [[TMP0:%.*]]) #[[ATTR0]] {
// CHECK-C: entry:
//
-// CHECK-CXX-LABEL: define dso_local float @_Z7test_s82s8
-// CHECK-CXX-SAME: (float [[TMP0:%.*]]) #[[ATTR0]] {
-// CHECK-CXX: entry:
+// CHECK32-CXX-LABEL: define dso_local i32 @_Z7test_s82s8
+// CHECK32-CXX-SAME: (i32 [[A_COERCE:%.*]]) #[[ATTR0]] {
+// CHECK32-CXX: entry:
+//
+// CHECK64-CXX-LABEL: define dso_local i64 @_Z7test_s82s8
+// CHECK64-CXX-SAME: (i64 [[A_COERCE:%.*]]) #[[ATTR0]] {
+// CHECK64-CXX: entry:
//
struct s8 test_s8(struct s8 a) {
return a;
More information about the cfe-commits
mailing list