This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG17a58b3ca7ec: [clang][RISCV] Fix ABI handling of empty structs with hard FP calling… (authored by asb). Herald added a subscriber: wangpc.
Changed prior to commit: https://reviews.llvm.org/D142327?vs=491781&id=543432#toc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142327/new/ https://reviews.llvm.org/D142327 Files: 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
Index: clang/test/CodeGen/RISCV/abi-empty-structs.c =================================================================== --- clang/test/CodeGen/RISCV/abi-empty-structs.c +++ clang/test/CodeGen/RISCV/abi-empty-structs.c @@ -19,8 +19,9 @@ #include <stdint.h> // Fields containing empty structs or unions are ignored when flattening -// structs for the hard FP ABIs, even in C++. -// FIXME: This isn't currently respected. +// 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>. struct empty { struct { struct { } e; }; }; struct s1 { struct empty e; float f; }; @@ -29,13 +30,9 @@ // CHECK-C-SAME: (float [[TMP0:%.*]]) #[[ATTR0:[0-9]+]] { // CHECK-C: 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: +// CHECK-CXX-LABEL: define dso_local float @_Z7test_s12s1 +// CHECK-CXX-SAME: (float [[TMP0:%.*]]) #[[ATTR0:[0-9]+]] { +// CHECK-CXX: entry: // struct s1 test_s1(struct s1 a) { return a; @@ -47,13 +44,9 @@ // CHECK-C-SAME: (i32 [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] { // CHECK-C: 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: +// CHECK-CXX-LABEL: define dso_local { i32, float } @_Z7test_s22s2 +// CHECK-CXX-SAME: (i32 [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] { +// CHECK-CXX: entry: // struct s2 test_s2(struct s2 a) { return a; @@ -65,13 +58,9 @@ // CHECK-C-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] { // CHECK-C: 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: +// CHECK-CXX-LABEL: define dso_local { float, float } @_Z7test_s32s3 +// CHECK-CXX-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] { +// CHECK-CXX: entry: // struct s3 test_s3(struct s3 a) { return a; @@ -83,13 +72,9 @@ // CHECK-C-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] { // CHECK-C: 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: +// CHECK-CXX-LABEL: define dso_local { float, float } @_Z7test_s42s4 +// CHECK-CXX-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) #[[ATTR0]] { +// CHECK-CXX: entry: // struct s4 test_s4(struct s4 a) { return a; @@ -142,7 +127,7 @@ // CHECK-C: entry: // // CHECK-CXX-LABEL: define dso_local float @_Z7test_s72s7 -// CHECK-CXX-SAME: (float [[TMP0:%.*]]) #[[ATTR0:[0-9]+]] { +// CHECK-CXX-SAME: (float [[TMP0:%.*]]) #[[ATTR0]] { // CHECK-CXX: entry: // struct s7 test_s7(struct s7 a) { @@ -156,13 +141,9 @@ // CHECK-C-SAME: (float [[TMP0:%.*]]) #[[ATTR0]] { // CHECK-C: 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: +// CHECK-CXX-LABEL: define dso_local float @_Z7test_s82s8 +// CHECK-CXX-SAME: (float [[TMP0:%.*]]) #[[ATTR0]] { +// CHECK-CXX: entry: // struct s8 test_s8(struct s8 a) { return a; Index: clang/lib/CodeGen/Targets/RISCV.cpp =================================================================== --- clang/lib/CodeGen/Targets/RISCV.cpp +++ clang/lib/CodeGen/Targets/RISCV.cpp @@ -152,6 +152,13 @@ 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, @@ -168,7 +175,7 @@ // copy constructor are not eligible for the FP calling convention. if (getRecordArgABI(Ty, CGT.getCXXABI())) return false; - if (isEmptyRecord(getContext(), Ty, true)) + if (isEmptyRecord(getContext(), Ty, true, true)) return true; const RecordDecl *RD = RTy->getDecl(); // Unions aren't eligible unless they're empty (which is caught above). Index: clang/lib/CodeGen/ABIInfoImpl.h =================================================================== --- clang/lib/CodeGen/ABIInfoImpl.h +++ clang/lib/CodeGen/ABIInfoImpl.h @@ -122,13 +122,19 @@ 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). -bool isEmptyField(ASTContext &Context, const FieldDecl *FD, bool AllowArrays); +/// 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); /// isEmptyRecord - Return true iff a structure contains only empty /// fields. Note that a structure with a flexible array member is not -/// considered empty. -bool isEmptyRecord(ASTContext &Context, QualType T, bool AllowArrays); +/// 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); /// isSingleElementStruct - Determine if a structure is a "single /// element struct", i.e. it has exactly one non-empty field or Index: clang/lib/CodeGen/ABIInfoImpl.cpp =================================================================== --- clang/lib/CodeGen/ABIInfoImpl.cpp +++ clang/lib/CodeGen/ABIInfoImpl.cpp @@ -246,7 +246,7 @@ } bool CodeGen::isEmptyField(ASTContext &Context, const FieldDecl *FD, - bool AllowArrays) { + bool AllowArrays, bool AsIfNoUniqueAddr) { if (FD->isUnnamedBitfield()) return true; @@ -280,13 +280,14 @@ // not arrays of records, so we must also check whether we stripped off an // array type above. if (isa<CXXRecordDecl>(RT->getDecl()) && - (WasArray || !FD->hasAttr<NoUniqueAddressAttr>())) + (WasArray || (!AsIfNoUniqueAddr && !FD->hasAttr<NoUniqueAddressAttr>()))) return false; - return isEmptyRecord(Context, FT, AllowArrays); + return isEmptyRecord(Context, FT, AllowArrays, AsIfNoUniqueAddr); } -bool CodeGen::isEmptyRecord(ASTContext &Context, QualType T, bool AllowArrays) { +bool CodeGen::isEmptyRecord(ASTContext &Context, QualType T, bool AllowArrays, + bool AsIfNoUniqueAddr) { const RecordType *RT = T->getAs<RecordType>(); if (!RT) return false; @@ -297,11 +298,11 @@ // 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)) + if (!isEmptyRecord(Context, I.getType(), true, AsIfNoUniqueAddr)) return false; for (const auto *I : RD->fields()) - if (!isEmptyField(Context, I, AllowArrays)) + if (!isEmptyField(Context, I, AllowArrays, AsIfNoUniqueAddr)) return false; return true; } Index: clang/docs/ReleaseNotes.rst =================================================================== --- clang/docs/ReleaseNotes.rst +++ clang/docs/ReleaseNotes.rst @@ -899,6 +899,9 @@ - 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 ^^^^^^^^^^^^^^^^^^^^^^^^^
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
