[llvm-branch-commits] [llvm] 3676ef1 - [ARM][GISel] Treat calls as variadic even if only fixed arguments provided
Author: Oliver Stannard Date: 2021-01-15T09:37:01Z New Revision: 3676ef105389f0a8fd7d0efa1477adc209f0b486 URL: https://github.com/llvm/llvm-project/commit/3676ef105389f0a8fd7d0efa1477adc209f0b486 DIFF: https://github.com/llvm/llvm-project/commit/3676ef105389f0a8fd7d0efa1477adc209f0b486.diff LOG: [ARM][GISel] Treat calls as variadic even if only fixed arguments provided For the ARM hard-float calling convention, calls to variadic functions need to be treated diffrently, even if only the fixed arguments are provided. This fixes GCC-C-execute-pr68390 in the test-suite, which is failing on the ARM GlobaISel bot. Added: Modified: llvm/lib/Target/ARM/ARMCallLowering.cpp llvm/test/CodeGen/ARM/GlobalISel/irtranslator-varargs-lowering.ll Removed: diff --git a/llvm/lib/Target/ARM/ARMCallLowering.cpp b/llvm/lib/Target/ARM/ARMCallLowering.cpp index 10d66fb00650..6feed82596cc 100644 --- a/llvm/lib/Target/ARM/ARMCallLowering.cpp +++ b/llvm/lib/Target/ARM/ARMCallLowering.cpp @@ -538,22 +538,18 @@ bool ARMCallLowering::lowerCall(MachineIRBuilder &MIRBuilder, CallLoweringInfo & MIB.addRegMask(TRI->getCallPreservedMask(MF, Info.CallConv)); - bool IsVarArg = false; SmallVector ArgInfos; for (auto Arg : Info.OrigArgs) { if (!isSupportedType(DL, TLI, Arg.Ty)) return false; -if (!Arg.IsFixed) - IsVarArg = true; - if (Arg.Flags[0].isByVal()) return false; splitToValueTypes(Arg, ArgInfos, MF); } - auto ArgAssignFn = TLI.CCAssignFnForCall(Info.CallConv, IsVarArg); + auto ArgAssignFn = TLI.CCAssignFnForCall(Info.CallConv, Info.IsVarArg); ARMOutgoingValueHandler ArgHandler(MIRBuilder, MRI, MIB, ArgAssignFn); if (!handleAssignments(MIRBuilder, ArgInfos, ArgHandler)) return false; @@ -567,7 +563,7 @@ bool ARMCallLowering::lowerCall(MachineIRBuilder &MIRBuilder, CallLoweringInfo & ArgInfos.clear(); splitToValueTypes(Info.OrigRet, ArgInfos, MF); -auto RetAssignFn = TLI.CCAssignFnForReturn(Info.CallConv, IsVarArg); +auto RetAssignFn = TLI.CCAssignFnForReturn(Info.CallConv, Info.IsVarArg); CallReturnHandler RetHandler(MIRBuilder, MRI, MIB, RetAssignFn); if (!handleAssignments(MIRBuilder, ArgInfos, RetHandler)) return false; diff --git a/llvm/test/CodeGen/ARM/GlobalISel/irtranslator-varargs-lowering.ll b/llvm/test/CodeGen/ARM/GlobalISel/irtranslator-varargs-lowering.ll index 98f1c1c8386b..62d70fe36a2a 100644 --- a/llvm/test/CodeGen/ARM/GlobalISel/irtranslator-varargs-lowering.ll +++ b/llvm/test/CodeGen/ARM/GlobalISel/irtranslator-varargs-lowering.ll @@ -59,6 +59,27 @@ entry: ret float %r } +define arm_aapcs_vfpcc float @test_call_to_varargs_with_floats_fixed_args_only(float %a, double %b) { +; CHECK-LABEL: name: test_call_to_varargs_with_floats_fixed_args_only +; CHECK-DAG: [[AVREG:%[0-9]+]]:_(s32) = COPY $s0 +; CHECK-DAG: [[BVREG:%[0-9]+]]:_(s64) = COPY $d1 +; CHECK: ADJCALLSTACKDOWN 0, 0, 14 /* CC::al */, $noreg, implicit-def $sp, implicit $sp +; CHECK-DAG: $r0 = COPY [[AVREG]] +; CHECK-DAG: [[B1:%[0-9]+]]:_(s32), [[B2:%[0-9]+]]:_(s32) = G_UNMERGE_VALUES [[BVREG]](s64) +; CHECK-DAG: $r2 = COPY [[B1]] +; CHECK-DAG: $r3 = COPY [[B2]] +; ARM: BL @float_varargs_target, csr_aapcs, implicit-def $lr, implicit $sp, implicit $r0, implicit $r2, implicit $r3, implicit-def $r0 +; THUMB: tBL 14 /* CC::al */, $noreg, @float_varargs_target, csr_aapcs, implicit-def $lr, implicit $sp, implicit $r0, implicit $r2, implicit $r3, implicit-def $r0 +; CHECK: [[RVREG:%[0-9]+]]:_(s32) = COPY $r0 +; CHECK: ADJCALLSTACKUP 0, 0, 14 /* CC::al */, $noreg, implicit-def $sp, implicit $sp +; CHECK: $s0 = COPY [[RVREG]] +; ARM: BX_RET 14 /* CC::al */, $noreg, implicit $s0 +; THUMB: tBX_RET 14 /* CC::al */, $noreg, implicit $s0 +entry: + %r = notail call arm_aapcs_vfpcc float(float, double, ...) @float_varargs_target(float %a, double %b) + ret float %r +} + define arm_aapcs_vfpcc float @test_indirect_call_to_varargs(float (float, double, ...) *%fptr, float %a, double %b) { ; CHECK-LABEL: name: test_indirect_call_to_varargs ; CHECK-DAG: [[FPTRVREG:%[0-9]+]]:gpr(p0) = COPY $r0 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [compiler-rt] 4839378 - Revert "[sanitizer] Define SANITIZER_GLIBC to refine SANITIZER_LINUX feature detection and support musl"
Author: Oliver Stannard Date: 2021-01-06T10:31:59Z New Revision: 4839378ca05f88faed53ea25457fd93fcad93460 URL: https://github.com/llvm/llvm-project/commit/4839378ca05f88faed53ea25457fd93fcad93460 DIFF: https://github.com/llvm/llvm-project/commit/4839378ca05f88faed53ea25457fd93fcad93460.diff LOG: Revert "[sanitizer] Define SANITIZER_GLIBC to refine SANITIZER_LINUX feature detection and support musl" This reverts commit b7718b617557aa9827f994a16267537236634095, because it is causing build failures on all 32-bit ARM bots which build compiler-rt. Added: Modified: compiler-rt/cmake/Modules/AddCompilerRT.cmake compiler-rt/lib/asan/asan_interceptors.h compiler-rt/lib/asan/tests/asan_test.cpp compiler-rt/lib/interception/interception_linux.cpp compiler-rt/lib/interception/interception_linux.h compiler-rt/lib/msan/tests/msan_test.cpp compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors_ioctl.inc compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp compiler-rt/lib/sanitizer_common/sanitizer_platform.h compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp Removed: diff --git a/compiler-rt/cmake/Modules/AddCompilerRT.cmake b/compiler-rt/cmake/Modules/AddCompilerRT.cmake index 361538a58e47..0b8db6a868a1 100644 --- a/compiler-rt/cmake/Modules/AddCompilerRT.cmake +++ b/compiler-rt/cmake/Modules/AddCompilerRT.cmake @@ -583,7 +583,6 @@ macro(add_custom_libcxx name prefix) CMAKE_OBJDUMP CMAKE_STRIP CMAKE_SYSROOT -LIBCXX_HAS_MUSL_LIBC PYTHON_EXECUTABLE Python3_EXECUTABLE Python2_EXECUTABLE diff --git a/compiler-rt/lib/asan/asan_interceptors.h b/compiler-rt/lib/asan/asan_interceptors.h index 45cdb80b1b64..4266a31cecb9 100644 --- a/compiler-rt/lib/asan/asan_interceptors.h +++ b/compiler-rt/lib/asan/asan_interceptors.h @@ -60,7 +60,7 @@ void InitializePlatformInterceptors(); # define ASAN_USE_ALIAS_ATTRIBUTE_FOR_INDEX 0 #endif -#if SANITIZER_GLIBC || SANITIZER_SOLARIS +#if (SANITIZER_LINUX && !SANITIZER_ANDROID) || SANITIZER_SOLARIS # define ASAN_INTERCEPT_SWAPCONTEXT 1 #else # define ASAN_INTERCEPT_SWAPCONTEXT 0 @@ -72,7 +72,7 @@ void InitializePlatformInterceptors(); # define ASAN_INTERCEPT_SIGLONGJMP 0 #endif -#if SANITIZER_GLIBC +#if SANITIZER_LINUX && !SANITIZER_ANDROID # define ASAN_INTERCEPT___LONGJMP_CHK 1 #else # define ASAN_INTERCEPT___LONGJMP_CHK 0 @@ -106,7 +106,7 @@ void InitializePlatformInterceptors(); # define ASAN_INTERCEPT_ATEXIT 0 #endif -#if SANITIZER_GLIBC +#if SANITIZER_LINUX && !SANITIZER_ANDROID # define ASAN_INTERCEPT___STRDUP 1 #else # define ASAN_INTERCEPT___STRDUP 0 diff --git a/compiler-rt/lib/asan/tests/asan_test.cpp b/compiler-rt/lib/asan/tests/asan_test.cpp index c0b79bba48ff..51a527359b49 100644 --- a/compiler-rt/lib/asan/tests/asan_test.cpp +++ b/compiler-rt/lib/asan/tests/asan_test.cpp @@ -804,7 +804,7 @@ char* MallocAndMemsetString(size_t size) { return MallocAndMemsetString(size, 'z'); } -#if SANITIZER_GLIBC +#if defined(__linux__) && !defined(__ANDROID__) #define READ_TEST(READ_N_BYTES) \ char *x = new char[10];\ int fd = open("/proc/self/stat", O_RDONLY);\ @@ -827,7 +827,7 @@ TEST(AddressSanitizer, pread64) { TEST(AddressSanitizer, read) { READ_TEST(read(fd, x, 15)); } -#endif // SANITIZER_GLIBC +#endif // defined(__linux__) && !defined(__ANDROID__) // This test case fails // Clang optimizes memcpy/memset calls which lead to unaligned access diff --git a/compiler-rt/lib/interception/interception_linux.cpp b/compiler-rt/lib/interception/interception_linux.cpp index 5111a87f0a6c..6883608d44f3 100644 --- a/compiler-rt/lib/interception/interception_linux.cpp +++ b/compiler-rt/lib/interception/interception_linux.cpp @@ -63,8 +63,8 @@ bool InterceptFunction(const char *name, uptr *ptr_to_real, uptr func, return addr && (func == wrapper); } -// dlvsym is a GNU extension supported by some other platforms. -#if SANITIZER_GLIBC || SANITIZER_FREEBSD || SANITIZER_NETBSD +// Android and Solaris do not have dlvsym +#if !SANITIZER_ANDROID && !SANITIZER_SOLARIS static void *GetFuncAddr(const char *name, const char *ver) { return dlvsym(RTLD_NEXT, name, ver); } @@ -75,7 +75,7 @@ bool InterceptFunction(const char *name, const char *ver, uptr *ptr_to_real, *ptr_to_real = (uptr)addr; return addr && (func == wrapper); } -#endif // SANITIZER_GLIBC || SANITIZER_FREEBSD || SANITIZER_NETBSD +#endif // !SANITIZER_ANDROID } // namespace __interception diff --git a/compiler-rt/lib/interception/interception_linux.h
[llvm-branch-commits] [llvm] 76f6b12 - Revert "[llvm] Use BasicBlock::phis() (NFC)"
Author: Oliver Stannard Date: 2021-01-07T09:43:33Z New Revision: 76f6b125cef1f5d949cd8f4049b14f572ecd9ee6 URL: https://github.com/llvm/llvm-project/commit/76f6b125cef1f5d949cd8f4049b14f572ecd9ee6 DIFF: https://github.com/llvm/llvm-project/commit/76f6b125cef1f5d949cd8f4049b14f572ecd9ee6.diff LOG: Revert "[llvm] Use BasicBlock::phis() (NFC)" Reverting because this causes crashes on the 2-stage buildbots, for example http://lab.llvm.org:8011/#/builders/7/builds/1140. This reverts commit 9b228f107d43341ef73af92865f73a9a076c5a76. Added: Modified: llvm/lib/IR/BasicBlock.cpp llvm/lib/Target/Hexagon/HexagonLoopIdiomRecognition.cpp llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp llvm/lib/Transforms/Utils/InlineFunction.cpp llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp Removed: diff --git a/llvm/lib/IR/BasicBlock.cpp b/llvm/lib/IR/BasicBlock.cpp index 688211877d18..7f34565f5cd8 100644 --- a/llvm/lib/IR/BasicBlock.cpp +++ b/llvm/lib/IR/BasicBlock.cpp @@ -440,8 +440,12 @@ BasicBlock *BasicBlock::splitBasicBlockBefore(iterator I, const Twine &BBName) { void BasicBlock::replacePhiUsesWith(BasicBlock *Old, BasicBlock *New) { // N.B. This might not be a complete BasicBlock, so don't assume // that it ends with a non-phi instruction. - for (PHINode &PN : phis()) -PN.replaceIncomingBlockWith(Old, New); + for (iterator II = begin(), IE = end(); II != IE; ++II) { +PHINode *PN = dyn_cast(II); +if (!PN) + break; +PN->replaceIncomingBlockWith(Old, New); + } } void BasicBlock::replaceSuccessorsPhiUsesWith(BasicBlock *Old, diff --git a/llvm/lib/Target/Hexagon/HexagonLoopIdiomRecognition.cpp b/llvm/lib/Target/Hexagon/HexagonLoopIdiomRecognition.cpp index 273d3ab97c7b..68c79d2a113f 100644 --- a/llvm/lib/Target/Hexagon/HexagonLoopIdiomRecognition.cpp +++ b/llvm/lib/Target/Hexagon/HexagonLoopIdiomRecognition.cpp @@ -2199,10 +2199,13 @@ bool HexagonLoopIdiomRecognize::processCopyingStore(Loop *CurLoop, if (ParentL) ParentL->addBasicBlockToLoop(NewPreheader, *LF); IRBuilder<>(NewPreheader).CreateBr(Header); -for (PHINode &PN : Header->phis()) { - int bx = PN.getBasicBlockIndex(Preheader); +for (auto &In : *Header) { + PHINode *PN = dyn_cast(&In); + if (!PN) +break; + int bx = PN->getBasicBlockIndex(Preheader); if (bx >= 0) -PN.setIncomingBlock(bx, NewPreheader); +PN->setIncomingBlock(bx, NewPreheader); } DT->addNewBlock(NewPreheader, Preheader); DT->changeImmediateDominator(Header, NewPreheader); diff --git a/llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp b/llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp index 060ca4bcd8ee..4cce05d595a8 100644 --- a/llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp +++ b/llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp @@ -483,20 +483,24 @@ static Optional analyzeLoopUnrollCost( // Prepare for the iteration by collecting any simplified entry or backedge // inputs. -for (PHINode &PHI : L->getHeader()->phis()) { +for (Instruction &I : *L->getHeader()) { + auto *PHI = dyn_cast(&I); + if (!PHI) +break; + // The loop header PHI nodes must have exactly two input: one from the // loop preheader and one from the loop latch. assert( - PHI.getNumIncomingValues() == 2 && + PHI->getNumIncomingValues() == 2 && "Must have an incoming value only for the preheader and the latch."); - Value *V = PHI.getIncomingValueForBlock( + Value *V = PHI->getIncomingValueForBlock( Iteration == 0 ? L->getLoopPreheader() : L->getLoopLatch()); Constant *C = dyn_cast(V); if (Iteration != 0 && !C) C = SimplifiedValues.lookup(V); if (C) -SimplifiedInputValues.push_back({&PHI, C}); +SimplifiedInputValues.push_back({PHI, C}); } // Now clear and re-populate the map for the next iteration. @@ -621,8 +625,12 @@ static Optional analyzeLoopUnrollCost( BasicBlock *ExitingBB, *ExitBB; std::tie(ExitingBB, ExitBB) = ExitWorklist.pop_back_val(); -for (PHINode &PN : ExitBB->phis()) { - Value *Op = PN.getIncomingValueForBlock(ExitingBB); +for (Instruction &I : *ExitBB) { + auto *PN = dyn_cast(&I); + if (!PN) +break; + + Value *Op = PN->getIncomingValueForBlock(ExitingBB); if (auto *OpI = dyn_cast(Op)) if (L->contains(OpI)) AddCostRecursively(*OpI, TripCount - 1); diff --git a/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp b/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp index ed3f87a7d0e7..68ddebf113d1 100644 --- a/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp +++ b/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp @@ -2843,8 +2843,12 @@ static void computeLiveInValues(Basic
[llvm-branch-commits] [llvm] 7d04e70 - [Lit] Fix flaky test on heavily loaded bots
Author: Oliver Stannard Date: 2020-12-07T09:05:55Z New Revision: 7d04e70627aa0734ce820bec0dcb7fa28a9ea457 URL: https://github.com/llvm/llvm-project/commit/7d04e70627aa0734ce820bec0dcb7fa28a9ea457 DIFF: https://github.com/llvm/llvm-project/commit/7d04e70627aa0734ce820bec0dcb7fa28a9ea457.diff LOG: [Lit] Fix flaky test on heavily loaded bots On some of the slow or heavily-loaded bots, this test was failing intermittently because the infinite_loop.py script might not emit anything to stdout before the 1 second timeout, so the "Command Output" line isn't present in the output. That output isn't really important to this test, we just care that the process is killed, so we can just rmove that check line from the test. Differential revision: https://reviews.llvm.org/D92563 Added: Modified: llvm/utils/lit/tests/shtest-timeout.py Removed: diff --git a/llvm/utils/lit/tests/shtest-timeout.py b/llvm/utils/lit/tests/shtest-timeout.py index 558aca5f05bf..f229c286088b 100644 --- a/llvm/utils/lit/tests/shtest-timeout.py +++ b/llvm/utils/lit/tests/shtest-timeout.py @@ -42,7 +42,6 @@ # CHECK-OUT-COMMON: TIMEOUT: per_test_timeout :: infinite_loop.py # CHECK-OUT-COMMON: Timeout: Reached timeout of 1 seconds -# CHECK-OUT-COMMON: Command {{([0-9]+ )?}}Output # CHECK-OUT-COMMON: Timed Out: 1 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/20.x: [ARM] Empty structs are 1-byte for C++ ABI (#124762) (PR #125191)
https://github.com/ostannard closed https://github.com/llvm/llvm-project/pull/125191 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/20.x: [ARM] Empty structs are 1-byte for C++ ABI (#124762) (PR #125191)
ostannard wrote: This needs release notes adding, and I can't add commits to an llvmbot PR, so I'll abandon this and create my own backport PR. https://github.com/llvm/llvm-project/pull/125191 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/20.x: [ARM] Empty structs are 1-byte for C++ ABI (#124762) (PR #125194)
https://github.com/ostannard milestoned https://github.com/llvm/llvm-project/pull/125194 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/20.x: [ARM] Empty structs are 1-byte for C++ ABI (#124762) (PR #125194)
https://github.com/ostannard created https://github.com/llvm/llvm-project/pull/125194 Backport 97b066f4e92a0df46b9d10721e988210f0d1afb6 Add release note. >From f486aca315b8af06e050fab2f7d4f31675607b07 Mon Sep 17 00:00:00 2001 From: Oliver Stannard Date: Fri, 31 Jan 2025 09:03:01 + Subject: [PATCH 1/2] [ARM] Empty structs are 1-byte for C++ ABI (#124762) For C++ (but not C), empty structs should be passed to functions as if they are a 1 byte object with 1 byte alignment. This is defined in Arm's CPPABI32: https://github.com/ARM-software/abi-aa/blob/main/cppabi32/cppabi32.rst For the purposes of parameter passing in AAPCS32, a parameter whose type is an empty class shall be treated as if its type were an aggregate with a single member of type unsigned byte. The AArch64 equivalent of this has an exception for structs containing an array of size zero, I've kept that logic for ARM. I've not found a reason for this exception, but I've checked that GCC does have the same behaviour for ARM as it does for AArch64. The AArch64 version has an Apple ABI with different rules, which ignores empty structs in both C and C++. This is documented at https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms. The ARM equivalent of that appears to be AAPCS16_VFP, used for WatchOS, but I can't find any documentation for that ABI, so I'm not sure what rules it should follow. For now I've left it following the AArch64 Apple rules. --- clang/include/clang/Basic/LangOptions.h | 2 + clang/lib/CodeGen/Targets/ARM.cpp | 45 +++- clang/test/CodeGen/arm-empty-args.cpp | 131 3 files changed, 173 insertions(+), 5 deletions(-) create mode 100644 clang/test/CodeGen/arm-empty-args.cpp diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h index 114a5d34a008bd..16c35bcf49339c 100644 --- a/clang/include/clang/Basic/LangOptions.h +++ b/clang/include/clang/Basic/LangOptions.h @@ -246,6 +246,8 @@ class LangOptionsBase { /// construction vtable because it hasn't added 'type' as a substitution. /// - Skip mangling enclosing class templates of member-like friend /// function templates. +/// - Ignore empty struct arguments in C++ mode for ARM, instead of +/// passing them as if they had a size of 1 byte. Ver19, /// Conform to the underlying platform's C and C++ ABIs as closely diff --git a/clang/lib/CodeGen/Targets/ARM.cpp b/clang/lib/CodeGen/Targets/ARM.cpp index 2d858fa2f3c3a3..47e31ceeaf2943 100644 --- a/clang/lib/CodeGen/Targets/ARM.cpp +++ b/clang/lib/CodeGen/Targets/ARM.cpp @@ -71,6 +71,7 @@ class ARMABIInfo : public ABIInfo { unsigned functionCallConv) const; ABIArgInfo classifyHomogeneousAggregate(QualType Ty, const Type *Base, uint64_t Members) const; + bool shouldIgnoreEmptyArg(QualType Ty) const; ABIArgInfo coerceIllegalVector(QualType Ty) const; bool isIllegalVectorType(QualType Ty) const; bool containsAnyFP16Vectors(QualType Ty) const; @@ -328,6 +329,31 @@ ABIArgInfo ARMABIInfo::classifyHomogeneousAggregate(QualType Ty, return ABIArgInfo::getDirect(nullptr, 0, nullptr, false, Align); } +bool ARMABIInfo::shouldIgnoreEmptyArg(QualType Ty) const { + uint64_t Size = getContext().getTypeSize(Ty); + assert((isEmptyRecord(getContext(), Ty, true) || Size == 0) && + "Arg is not empty"); + + // Empty records are ignored in C mode, and in C++ on WatchOS. + if (!getContext().getLangOpts().CPlusPlus || + getABIKind() == ARMABIKind::AAPCS16_VFP) +return true; + + // In C++ mode, arguments which have sizeof() == 0 are ignored. This is not a + // situation which is defined by any C++ standard or ABI, but this matches + // GCC's de facto ABI. + if (Size == 0) +return true; + + // Clang 19.0 and earlier always ignored empty struct arguments in C++ mode. + if (getContext().getLangOpts().getClangABICompat() <= + LangOptions::ClangABI::Ver19) +return true; + + // Otherwise, they are passed as if they have a size of 1 byte. + return false; +} + ABIArgInfo ARMABIInfo::classifyArgumentType(QualType Ty, bool isVariadic, unsigned functionCallConv) const { // 6.1.2.1 The following argument types are VFP CPRCs: @@ -366,9 +392,15 @@ ABIArgInfo ARMABIInfo::classifyArgumentType(QualType Ty, bool isVariadic, return getNaturalAlignIndirect(Ty, RAA == CGCXXABI::RAA_DirectInMemory); } - // Ignore empty records. - if (isEmptyRecord(getContext(), Ty, true)) -return ABIArgInfo::getIgnore(); + // Empty records are either ignored completely or passed as if they were a + // 1-byte object, depending on the ABI and language standard. + if (isEmptyRecord(getContext(), Ty, true) || + getContext().getTypeSize(Ty) == 0) { +if (shouldIgnoreEmptyArg(Ty)) + return ABIArgInfo::getIgnore();
[llvm-branch-commits] [clang] release/20.x: [ARM] Empty structs are 1-byte for C++ ABI (#124762) (PR #125191)
ostannard wrote: Superseded by #125194. https://github.com/llvm/llvm-project/pull/125191 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/20.x: [ARM] Empty structs are 1-byte for C++ ABI (#124762) (PR #125194)
https://github.com/ostannard edited https://github.com/llvm/llvm-project/pull/125194 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/20.x: [ARM] Empty structs are 1-byte for C++ ABI (#124762) (PR #125194)
https://github.com/ostannard updated https://github.com/llvm/llvm-project/pull/125194 >From f486aca315b8af06e050fab2f7d4f31675607b07 Mon Sep 17 00:00:00 2001 From: Oliver Stannard Date: Fri, 31 Jan 2025 09:03:01 + Subject: [PATCH 1/2] [ARM] Empty structs are 1-byte for C++ ABI (#124762) For C++ (but not C), empty structs should be passed to functions as if they are a 1 byte object with 1 byte alignment. This is defined in Arm's CPPABI32: https://github.com/ARM-software/abi-aa/blob/main/cppabi32/cppabi32.rst For the purposes of parameter passing in AAPCS32, a parameter whose type is an empty class shall be treated as if its type were an aggregate with a single member of type unsigned byte. The AArch64 equivalent of this has an exception for structs containing an array of size zero, I've kept that logic for ARM. I've not found a reason for this exception, but I've checked that GCC does have the same behaviour for ARM as it does for AArch64. The AArch64 version has an Apple ABI with different rules, which ignores empty structs in both C and C++. This is documented at https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms. The ARM equivalent of that appears to be AAPCS16_VFP, used for WatchOS, but I can't find any documentation for that ABI, so I'm not sure what rules it should follow. For now I've left it following the AArch64 Apple rules. --- clang/include/clang/Basic/LangOptions.h | 2 + clang/lib/CodeGen/Targets/ARM.cpp | 45 +++- clang/test/CodeGen/arm-empty-args.cpp | 131 3 files changed, 173 insertions(+), 5 deletions(-) create mode 100644 clang/test/CodeGen/arm-empty-args.cpp diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h index 114a5d34a008bd7..16c35bcf49339c6 100644 --- a/clang/include/clang/Basic/LangOptions.h +++ b/clang/include/clang/Basic/LangOptions.h @@ -246,6 +246,8 @@ class LangOptionsBase { /// construction vtable because it hasn't added 'type' as a substitution. /// - Skip mangling enclosing class templates of member-like friend /// function templates. +/// - Ignore empty struct arguments in C++ mode for ARM, instead of +/// passing them as if they had a size of 1 byte. Ver19, /// Conform to the underlying platform's C and C++ ABIs as closely diff --git a/clang/lib/CodeGen/Targets/ARM.cpp b/clang/lib/CodeGen/Targets/ARM.cpp index 2d858fa2f3c3a35..47e31ceeaf29431 100644 --- a/clang/lib/CodeGen/Targets/ARM.cpp +++ b/clang/lib/CodeGen/Targets/ARM.cpp @@ -71,6 +71,7 @@ class ARMABIInfo : public ABIInfo { unsigned functionCallConv) const; ABIArgInfo classifyHomogeneousAggregate(QualType Ty, const Type *Base, uint64_t Members) const; + bool shouldIgnoreEmptyArg(QualType Ty) const; ABIArgInfo coerceIllegalVector(QualType Ty) const; bool isIllegalVectorType(QualType Ty) const; bool containsAnyFP16Vectors(QualType Ty) const; @@ -328,6 +329,31 @@ ABIArgInfo ARMABIInfo::classifyHomogeneousAggregate(QualType Ty, return ABIArgInfo::getDirect(nullptr, 0, nullptr, false, Align); } +bool ARMABIInfo::shouldIgnoreEmptyArg(QualType Ty) const { + uint64_t Size = getContext().getTypeSize(Ty); + assert((isEmptyRecord(getContext(), Ty, true) || Size == 0) && + "Arg is not empty"); + + // Empty records are ignored in C mode, and in C++ on WatchOS. + if (!getContext().getLangOpts().CPlusPlus || + getABIKind() == ARMABIKind::AAPCS16_VFP) +return true; + + // In C++ mode, arguments which have sizeof() == 0 are ignored. This is not a + // situation which is defined by any C++ standard or ABI, but this matches + // GCC's de facto ABI. + if (Size == 0) +return true; + + // Clang 19.0 and earlier always ignored empty struct arguments in C++ mode. + if (getContext().getLangOpts().getClangABICompat() <= + LangOptions::ClangABI::Ver19) +return true; + + // Otherwise, they are passed as if they have a size of 1 byte. + return false; +} + ABIArgInfo ARMABIInfo::classifyArgumentType(QualType Ty, bool isVariadic, unsigned functionCallConv) const { // 6.1.2.1 The following argument types are VFP CPRCs: @@ -366,9 +392,15 @@ ABIArgInfo ARMABIInfo::classifyArgumentType(QualType Ty, bool isVariadic, return getNaturalAlignIndirect(Ty, RAA == CGCXXABI::RAA_DirectInMemory); } - // Ignore empty records. - if (isEmptyRecord(getContext(), Ty, true)) -return ABIArgInfo::getIgnore(); + // Empty records are either ignored completely or passed as if they were a + // 1-byte object, depending on the ABI and language standard. + if (isEmptyRecord(getContext(), Ty, true) || + getContext().getTypeSize(Ty) == 0) { +if (shouldIgnoreEmptyArg(Ty)) + return ABIArgInfo::getIgnore(); +else + return ABIArgInfo::getDirect(llvm::Type::getInt8T