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; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
