diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -196,6 +196,9 @@ ^^^^^^^^^^^^^^ - Unaligned memory accesses can be toggled by ``-m[no-]unaligned-access`` or the aliases ``-m[no-]strict-align``. +- 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.h b/clang/lib/CodeGen/ABIInfoImpl.h --- a/clang/lib/CodeGen/ABIInfoImpl.h +++ b/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 diff --git a/clang/lib/CodeGen/ABIInfoImpl.cpp b/clang/lib/CodeGen/ABIInfoImpl.cpp --- a/clang/lib/CodeGen/ABIInfoImpl.cpp +++ b/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(RT->getDecl()) && - (WasArray || !FD->hasAttr())) + (WasArray || (!AsIfNoUniqueAddr && !FD->hasAttr()))) 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(); if (!RT) return false; @@ -297,11 +298,11 @@ // If this is a C++ record, check the bases first. if (const CXXRecordDecl *CXXRD = dyn_cast(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; } diff --git a/clang/lib/CodeGen/Targets/RISCV.cpp b/clang/lib/CodeGen/Targets/RISCV.cpp --- a/clang/lib/CodeGen/Targets/RISCV.cpp +++ b/clang/lib/CodeGen/Targets/RISCV.cpp @@ -151,6 +151,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()) { + if (ArraySize != 0 && isa(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, @@ -167,7 +174,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). @@ -237,6 +244,8 @@ NeededArgFPRs = 0; bool IsCandidate = detectFPCCEligibleStructHelper( Ty, CharUnits::Zero(), Field1Ty, Field1Off, Field2Ty, Field2Off); + if (!Field1Ty) + return false; // Not really a candidate if we have a single int but no float. if (Field1Ty && !Field2Ty && !Field1Ty->isFloatingPointTy()) return false; diff --git a/clang/test/CodeGen/RISCV/abi-empty-structs.c b/clang/test/CodeGen/RISCV/abi-empty-structs.c --- a/clang/test/CodeGen/RISCV/abi-empty-structs.c +++ b/clang/test/CodeGen/RISCV/abi-empty-structs.c @@ -1,4 +1,4 @@ -// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --function-signature --full-function-signature --filter "^define |^entry:" +// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --filter "^define |^entry:" --version 2 // RUN: %clang_cc1 -triple riscv32 -target-feature +f -target-abi ilp32f -emit-llvm %s -o - \ // RUN: | FileCheck -check-prefixes=CHECK-C,CHECK32-C %s // RUN: %clang_cc1 -triple riscv32 -target-feature +f -target-feature +d -target-abi ilp32d -emit-llvm %s -o - \ @@ -19,8 +19,9 @@ #include // 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 +// . 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,17 +141,31 @@ // CHECK-C-SAME: (float [[TMP0:%.*]]) #[[ATTR0]] { // CHECK-C: entry: // -// CHECK32-CXX-LABEL: define dso_local i32 @_Z7test_s82s8 +// 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; +} + +struct s9 { + struct empty e; +}; + +// CHECK-C-LABEL: define dso_local void @test_s9 +// CHECK-C-SAME: () #[[ATTR0]] { +// CHECK-C: entry: +// +// CHECK32-CXX-LABEL: define dso_local void @_Z7test_s92s9 // CHECK32-CXX-SAME: (i32 [[A_COERCE:%.*]]) #[[ATTR0]] { // CHECK32-CXX: entry: // -// CHECK64-CXX-LABEL: define dso_local i64 @_Z7test_s82s8 +// CHECK64-CXX-LABEL: define dso_local void @_Z7test_s92s9 // CHECK64-CXX-SAME: (i64 [[A_COERCE:%.*]]) #[[ATTR0]] { // CHECK64-CXX: entry: // -struct s8 test_s8(struct s8 a) { - return a; -} +void test_s9(struct s9 a) {} //// NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line: // CHECK32-C: {{.*}}