[llvm-branch-commits] [llvm] 3676ef1 - [ARM][GISel] Treat calls as variadic even if only fixed arguments provided

2021-01-15 Thread Oliver Stannard via llvm-branch-commits

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"

2021-01-06 Thread Oliver Stannard via llvm-branch-commits

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)"

2021-01-07 Thread Oliver Stannard via llvm-branch-commits

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

2020-12-07 Thread Oliver Stannard via llvm-branch-commits

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)

2025-01-31 Thread Oliver Stannard via llvm-branch-commits

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)

2025-01-31 Thread Oliver Stannard via llvm-branch-commits

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)

2025-01-31 Thread Oliver Stannard via llvm-branch-commits

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)

2025-01-31 Thread Oliver Stannard via llvm-branch-commits

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)

2025-01-31 Thread Oliver Stannard via llvm-branch-commits

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)

2025-01-31 Thread Oliver Stannard via llvm-branch-commits

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)

2025-02-10 Thread Oliver Stannard via llvm-branch-commits

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