carwil updated this revision to Diff 190653. carwil marked an inline comment as done. carwil added a comment.
I got bit a confused earlier. That does actually seem like correct behaviour (once we're no longer able to treat the struct as a homogeneous aggregate). I've tightened up the conditional a bit as it wouldn't have previously accounted for the case where you had mfloat-abi=hard with an AAPCS (non VFP) attribute. Although in this instance, the backend does seem to be correcting/ignoring that, it's still best we don't blindly rely on such behaviour. I've also re-structured the conditional and added some comments so it's (hopefully) a little bit clearer what's going on. I believe this fixes the bug at hand. There's some other calls to getABIKind referencing AAPCS16_VFP, but that doesn't have a direct translation to an LLVM/FI CallingConvention (that I can see). Seems there is a lot of different ways to check various ABI/CC's and none of them seem able to give you the full picture at any point. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59094/new/ https://reviews.llvm.org/D59094 Files: clang/lib/CodeGen/TargetInfo.cpp clang/test/CodeGenCXX/arm-pcs.cpp
Index: clang/test/CodeGenCXX/arm-pcs.cpp =================================================================== --- /dev/null +++ clang/test/CodeGenCXX/arm-pcs.cpp @@ -0,0 +1,51 @@ +// Covers a bug fix for ABI selection with homogenous aggregates: +// See: https://bugs.llvm.org/show_bug.cgi?id=39982 + +// REQUIRES: arm-registered-target +// RUN: %clang -mfloat-abi=hard --target=armv7-unknown-linux-gnueabi -O3 -S -o - %s | FileCheck %s -check-prefixes=HARD,CHECK +// RUN: %clang -mfloat-abi=softfp --target=armv7-unknown-linux-gnueabi -O3 -S -o - %s | FileCheck %s -check-prefixes=SOFTFP,CHECK +// RUN: %clang -mfloat-abi=soft --target=armv7-unknown-linux-gnueabi -O3 -S -o - %s | FileCheck %s -check-prefixes=SOFT,CHECK + +struct S { + float f; + float d; + float c; + float t; +}; + +// Variadic functions should always marshal for the base standard. +// See section 5.5 (Parameter Passing) of the AAPCS. +float __attribute__((pcs("aapcs-vfp"))) variadic(S s, ...) { + // CHECK-NOT: vmov s{{[0-9]+}}, s{{[0-9]+}} + // CHECK: mov r{{[0-9]+}}, r{{[0-9]+}} + return s.d; +} + +float no_attribute(S s) { + // SOFT: mov r{{[0-9]+}}, r{{[0-9]+}} + // SOFTFP: mov r{{[0-9]+}}, r{{[0-9]+}} + // HARD: vmov.f32 s{{[0-9]+}}, s{{[0-9]+}} + return s.d; +} + +float __attribute__((pcs("aapcs-vfp"))) baz(float x, float y) { + // CHECK-NOT: mov s{{[0-9]+}}, r{{[0-9]+}} + // SOFT: mov r{{[0-9]+}}, r{{[0-9]+}} + // SOFTFP: vmov.f32 s{{[0-9]+}}, s{{[0-9]+}} + // HARD: vmov.f32 s{{[0-9]+}}, s{{[0-9]+}} + return y; +} + +float __attribute__((pcs("aapcs-vfp"))) foo(S s) { + // CHECK-NOT: mov s{{[0-9]+}}, r{{[0-9]+}} + // SOFT: mov r{{[0-9]+}}, r{{[0-9]+}} + // SOFTFP: vmov.f32 s{{[0-9]+}}, s{{[0-9]+}} + // HARD: vmov.f32 s{{[0-9]+}}, s{{[0-9]+}} + return s.d; +} + +float __attribute__((pcs("aapcs"))) bar(S s) { + // CHECK-NOT: vmov.f32 s{{[0-9]+}}, s{{[0-9]+}} + // CHECK: mov r{{[0-9]+}}, r{{[0-9]+}} + return s.d; +} Index: clang/lib/CodeGen/TargetInfo.cpp =================================================================== --- clang/lib/CodeGen/TargetInfo.cpp +++ clang/lib/CodeGen/TargetInfo.cpp @@ -10,7 +10,6 @@ // definition used to handle ABI compliancy. // //===----------------------------------------------------------------------===// - #include "TargetInfo.h" #include "ABIInfo.h" #include "CGBlocks.h" @@ -5591,8 +5590,10 @@ ABIKind getABIKind() const { return Kind; } private: - ABIArgInfo classifyReturnType(QualType RetTy, bool isVariadic) const; - ABIArgInfo classifyArgumentType(QualType RetTy, bool isVariadic) const; + ABIArgInfo classifyReturnType(QualType RetTy, bool isVariadic, + unsigned functionCallConv) const; + ABIArgInfo classifyArgumentType(QualType RetTy, bool isVariadic, + unsigned functionCallConv) const; ABIArgInfo classifyHomogeneousAggregate(QualType Ty, const Type *Base, uint64_t Members) const; ABIArgInfo coerceIllegalVector(QualType Ty) const; @@ -5722,11 +5723,13 @@ void ARMABIInfo::computeInfo(CGFunctionInfo &FI) const { if (!::classifyReturnType(getCXXABI(), FI, *this)) - FI.getReturnInfo() = - classifyReturnType(FI.getReturnType(), FI.isVariadic()); + FI.getReturnInfo() = classifyReturnType(FI.getReturnType(), FI.isVariadic(), + FI.getCallingConvention()); for (auto &I : FI.arguments()) - I.info = classifyArgumentType(I.type, FI.isVariadic()); + I.info = classifyArgumentType(I.type, FI.isVariadic(), + FI.getCallingConvention()); + // Always honor user-specified calling convention. if (FI.getCallingConvention() != llvm::CallingConv::C) @@ -5805,8 +5808,8 @@ return ABIArgInfo::getDirect(nullptr, 0, nullptr, false); } -ABIArgInfo ARMABIInfo::classifyArgumentType(QualType Ty, - bool isVariadic) const { +ABIArgInfo ARMABIInfo::classifyArgumentType(QualType Ty, bool isVariadic, + unsigned functionCallConv) const { // 6.1.2.1 The following argument types are VFP CPRCs: // A single-precision floating-point type (including promoted // half-precision types); A double-precision floating-point type; @@ -5814,7 +5817,16 @@ // with a Base Type of a single- or double-precision floating-point type, // 64-bit containerized vectors or 128-bit containerized vectors with one // to four Elements. - bool IsEffectivelyAAPCS_VFP = getABIKind() == AAPCS_VFP && !isVariadic; + bool IsEffectivelyAAPCS_VFP = false; + // Variadic functions should always marshal to the base standard. + if (!isVariadic) { + // Give precedence to user-specified calling conventions. + if (functionCallConv != llvm::CallingConv::C) + IsEffectivelyAAPCS_VFP = + (functionCallConv == llvm::CallingConv::ARM_AAPCS_VFP); + else + IsEffectivelyAAPCS_VFP = (getABIKind() == AAPCS_VFP); + } Ty = useFirstFieldIfTransparentUnion(Ty); @@ -6008,10 +6020,20 @@ return true; } -ABIArgInfo ARMABIInfo::classifyReturnType(QualType RetTy, - bool isVariadic) const { - bool IsEffectivelyAAPCS_VFP = - (getABIKind() == AAPCS_VFP || getABIKind() == AAPCS16_VFP) && !isVariadic; +ABIArgInfo ARMABIInfo::classifyReturnType(QualType RetTy, bool isVariadic, + unsigned functionCallConv) const { + + bool IsEffectivelyAAPCS_VFP = false; + // Variadic functions should always marshal to the base standard. + if (!isVariadic) { + // Give precedence to user-specified calling conventions. + if (functionCallConv != llvm::CallingConv::C) + IsEffectivelyAAPCS_VFP = + (functionCallConv == llvm::CallingConv::ARM_AAPCS_VFP); + else + IsEffectivelyAAPCS_VFP = + (getABIKind() == AAPCS_VFP || getABIKind() == AAPCS16_VFP); + } if (RetTy->isVoidType()) return ABIArgInfo::getIgnore();
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
