ARM: Report error for invalid use of AAPCS_VFP calling convention

[charles.baylis]

Summary

The AAPCS_VFP calling convention cannot be supported on targets
which do not have VFP, or on Thumb-1 targets, which cannot access
the VFP.

[joerg]

LGTM, but please cut down the test cases by hand. Most of the attributes and module flags should be unnecessary.

[t.p.northover]

What's the use case here? For people using __attribute__((pcs(...))) Clang should be producing the diagnostic (and I'm not even entirely convinced it should be an error, if your code only contains integer types there's not actually any problem).

Beyond that the tests don't need to be in separate files and contain an awful lot of Clang-generated cruft (the attributes, metadata, entry label and source_filename).

[charles.baylis]

In D34875#796780, @t.p.northover wrote:

What's the use case here? For people using __attribute__((pcs(...))) Clang should be producing the diagnostic (and I'm not even entirely convinced it should be an error, if your code only contains integer types there's not actually any problem).

While the main use case is clang, the current behaviour is undesirable. When configured for the hardfp ABI, llvm and clang will (for Thumb-1, and targets without VFP) silently produce code which conforms to the softfloat ABI instead.

Fixing the problem in clang still allows that undesirable behaviour in LLVM. It may also be necessary to fix it in clang for cleaner error reporting, unless there is a more delicate way to report the error here.

Beyond that the tests don't need to be in separate files

I think that they do, because report_fatal_error() causes compilation to terminate so the subsequent tests don't generate a message which can be checked.

and contain an awful lot of Clang-generated cruft (the attributes, metadata, entry label and source_filename).

Ack.

[t.p.northover]

I think that they do, because report_fatal_error() causes compilation to terminate so the subsequent tests don't generate a message which can be checked.

Ah yes. And that's also a really big indicator for why this should be handled in Clang. All you know from this is that at least one function, somewhere in your file (or codebase if doing LTO) has an aapcs_vfp attribute.

I get that silently doing SoftFP isn't great either so I'm not completely opposed to this change, I just think it's a pretty half-baked solution to the actual user-facing problem.

[charles.baylis]

Updates:
. removed cruft from new test cases
. adjusted existing test cases to avoid hardfp/Thumb-1 combination

It does seem like there ought to be a better way to report the error, eg MachineInstr::emitError() has some logic to acquire a LocCookie. I'm not sure if/how I can do something similar here. Any hints?

[rovka]

Hi, you could try looking into LLVMContext::emitError - it shouldn't be too hard to get hold of an LLVMContext, and that's what we use to report e.g. errors in inline asm in SelectionDAGBuilder.

[fhahn]

In D34875#796985, @t.p.northover wrote:

I think that they do, because report_fatal_error() causes compilation to terminate so the subsequent tests don't generate a message which can be checked.

Ah yes. And that's also a really big indicator for why this should be handled in Clang. All you know from this is that at least one function, somewhere in your file (or codebase if doing LTO) has an aapcs_vfp attribute.

I get that silently doing SoftFP isn't great either so I'm not completely opposed to this change, I just think it's a pretty half-baked solution to the actual user-facing problem.

I think TargetInfo in Clang has a checkCallingConvention function, which is implemented by ARMTargetInfo. A user facing error message should probably go there. We have access to the triple there, so we should be able to error on cases where the selected architecture only has soft floats. I am not sure if we have access to target-features there though, so we probably won't be able to handle all cases there.

We probably still need a warning/error in LLVM to cover the LTO case, e.g. when linking modules compiled to use the hardfp ABI into a target module with softfp only. IMO we should report that case, but ideally using LLVMContext::emitError as Diana suggested.

D35826 and D35569 are related changes, adding an error message to Clang when -marm is selected for Thumb-only architectures and arguing for an error message when ARM codegen is requested for a Thumb-only architecture in LLVM. Whatever we decide, I think both cases should be handled consistently.

[compnerd]

ARM ELF has an additional field that indicates the PCS used. With the frontend validating the input, wouldn't we be sure that each object file is valid. If there is a mismatch in the PCS, that should already be an error from the linker. Wouldn't the LTO case be handled similarly?

lib/Target/ARM/ARMISelLowering.cpp

CCAssignFn *ARMTargetLowering::CCAssignFnForNode(CallingConv::ID CC,
   switch (getEffectiveCallingConv(CC, isVarArg)) {
   default:
     llvm_unreachable("Unsupported calling convention");
   case CallingConv::ARM_APCS:
     return (Return ? RetCC_ARM_APCS : CC_ARM_APCS);
   case CallingConv::ARM_AAPCS:
     return (Return ? RetCC_ARM_AAPCS : CC_ARM_AAPCS);
   case CallingConv::ARM_AAPCS_VFP:
+    if (getSubtarget()->isThumb1Only() || !getSubtarget()->hasVFP2())
+      report_fatal_error("The aapcs_vfp calling convention is not supported on this target.");
     return (Return ? RetCC_ARM_AAPCS_VFP : CC_ARM_AAPCS_VFP);
   case CallingConv::Fast:
     return (Return ? RetFastCC_ARM_APCS : FastCC_ARM_APCS);
   case CallingConv::GHC:
     return (Return ? RetCC_ARM_APCS : CC_ARM_APCS_GHC);
   case CallingConv::PreserveMost:
     return (Return ? RetCC_ARM_AAPCS : CC_ARM_AAPCS);
   }

test/CodeGen/ARM/aapcs-vfp-call-float-arg.ll

+; RUN: not llc -mtriple=armv4--linux-gnueabi -mattr=-vfp2 %s -o - 2>&1 | FileCheck %s
+; RUN: not llc -mtriple=thumbv4--linux-gnueabi -mattr=-vfp2,-neon %s -o - 2>&1 | FileCheck %s
+; RUN: not llc -mtriple=thumbv4--linux-gnueabi -mattr=+vfp2,-neon %s -o - 2>&1 | FileCheck %s
+; RUN: llc -mtriple=armv4--linux-gnueabi -mattr=+vfp2 %s -o - 2>&1 | not FileCheck %s
+; RUN: llc -mtriple=thumbv7--linux-gnueabi -mattr=+vfp2 %s -o - 2>&1 | not FileCheck %s
+
+define arm_aapcscc void @g() {
+; CHECK: LLVM ERROR: The aapcs_vfp calling convention is not supported on this target.
+  call arm_aapcs_vfpcc void @f(float 1.0)
+  ret void
+}
+
+declare arm_aapcs_vfpcc void @f(float)

test/CodeGen/ARM/aapcs-vfp-call-float-result.ll

+; RUN: not llc -mtriple=armv4--linux-gnueabi -mattr=-vfp2 %s -o - 2>&1 | FileCheck %s
+; RUN: not llc -mtriple=thumbv4--linux-gnueabi -mattr=-vfp2,-neon %s -o - 2>&1 | FileCheck %s
+; RUN: not llc -mtriple=thumbv4--linux-gnueabi -mattr=+vfp2,-neon %s -o - 2>&1 | FileCheck %s
+; RUN: llc -mtriple=armv4--linux-gnueabi -mattr=+vfp2 %s -o - 2>&1 | not FileCheck %s
+; RUN: llc -mtriple=thumbv7--linux-gnueabi -mattr=+vfp2 %s -o - 2>&1 | not FileCheck %s
+
+define arm_aapcscc float @g() {
+; CHECK: LLVM ERROR: The aapcs_vfp calling convention is not supported on this target.
+  %call = call arm_aapcs_vfpcc float @f()
+  ret float %call
+}
+
+declare arm_aapcs_vfpcc float @f()

test/CodeGen/ARM/aapcs-vfp-def-float-arg.ll

+; RUN: not llc -mtriple=armv4--linux-gnueabi -mattr=-vfp2 %s -o - 2>&1 | FileCheck %s
+; RUN: not llc -mtriple=thumbv4--linux-gnueabi -mattr=-vfp2,-neon %s -o - 2>&1 | FileCheck %s
+; RUN: not llc -mtriple=thumbv4--linux-gnueabi -mattr=+vfp2,-neon %s -o - 2>&1 | FileCheck %s
+; RUN: llc -mtriple=armv4--linux-gnueabi -mattr=+vfp2 %s -o - 2>&1 | not FileCheck %s
+; RUN: llc -mtriple=thumbv7--linux-gnueabi -mattr=+vfp2 %s -o - 2>&1 | not FileCheck %s
+
+define arm_aapcs_vfpcc void @f(float %x) #0 {
+; CHECK: LLVM ERROR: The aapcs_vfp calling convention is not supported on this target.
+  %x.addr = alloca float, align 4
+  store float %x, float* %x.addr, align 4
+  ret void
+}

test/CodeGen/ARM/aapcs-vfp-def-float-result.ll

+; RUN: not llc -mtriple=armv4--linux-gnueabi -mattr=-vfp2 %s -o - 2>&1 | FileCheck %s
+; RUN: not llc -mtriple=thumbv4--linux-gnueabi -mattr=-vfp2,-neon %s -o - 2>&1 | FileCheck %s
+; RUN: not llc -mtriple=thumbv4--linux-gnueabi -mattr=+vfp2,-neon %s -o - 2>&1 | FileCheck %s
+; RUN: llc -mtriple=armv4--linux-gnueabi -mattr=+vfp2 %s -o - 2>&1 | not FileCheck %s
+; RUN: llc -mtriple=thumbv7--linux-gnueabi -mattr=+vfp2 %s -o - 2>&1 | not FileCheck %s
+
+define arm_aapcs_vfpcc float @f() {
+; CHECK: LLVM ERROR: The aapcs_vfp calling convention is not supported on this target.
+  ret float 1.0
+}

test/CodeGen/ARM/arm-eabi.ll

 ; RUN: llc < %s -mtriple=arm-none-eabi -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-EABI
-; RUN: llc < %s -mtriple=arm-none-eabihf -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-EABI
+; RUN: llc < %s -mtriple=arm-none-eabihf -mattr=+vfp2 -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-EABI
 ; RUN: llc < %s -mtriple=arm-none-androideabi -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-EABI
 ; RUN: llc < %s -mtriple=arm-none-gnueabi -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-GNUEABI
-; RUN: llc < %s -mtriple=arm-none-gnueabihf -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-GNUEABI
+; RUN: llc < %s -mtriple=arm-none-gnueabihf -mattr=+vfp2 -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-GNUEABI
 ; RUN: llc < %s -mtriple=arm-none-musleabi -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-GNUEABI
-; RUN: llc < %s -mtriple=arm-none-musleabihf -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-GNUEABI
+; RUN: llc < %s -mtriple=arm-none-musleabihf -mattr=+vfp2 -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-GNUEABI
 ; RUN: llc < %s -mtriple=arm-none-eabi -meabi=gnu -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-GNUEABI
-; RUN: llc < %s -mtriple=arm-none-eabihf -meabi=gnu -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-GNUEABI
+; RUN: llc < %s -mtriple=arm-none-eabihf -mattr=+vfp2 -meabi=gnu -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-GNUEABI
 ; RUN: llc < %s -mtriple=arm-none-androideabi -meabi=gnu -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-GNUEABI
 ; RUN: llc < %s -mtriple=arm-none-gnueabi -meabi=gnu -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-GNUEABI
-; RUN: llc < %s -mtriple=arm-none-gnueabihf -meabi=gnu -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-GNUEABI
+; RUN: llc < %s -mtriple=arm-none-gnueabihf -mattr=+vfp2 -meabi=gnu -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-GNUEABI
 ; RUN: llc < %s -mtriple=arm-none-musleabi -meabi=gnu -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-GNUEABI
-; RUN: llc < %s -mtriple=arm-none-musleabihf -meabi=gnu -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-GNUEABI
+; RUN: llc < %s -mtriple=arm-none-musleabihf -mattr=+vfp2 -meabi=gnu -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-GNUEABI
 ; RUN: llc < %s -mtriple=arm-none-eabi -meabi=4 -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-EABI
-; RUN: llc < %s -mtriple=arm-none-eabihf -meabi=4 -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-EABI
+; RUN: llc < %s -mtriple=arm-none-eabihf -mattr=+vfp2 -meabi=4 -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-EABI
 ; RUN: llc < %s -mtriple=arm-none-androideabi -meabi=4 -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-EABI
 ; RUN: llc < %s -mtriple=arm-none-gnueabi -meabi=4 -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-EABI
-; RUN: llc < %s -mtriple=arm-none-gnueabihf -meabi=4 -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-EABI
+; RUN: llc < %s -mtriple=arm-none-gnueabihf -mattr=+vfp2 -meabi=4 -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-EABI
 ; RUN: llc < %s -mtriple=arm-none-musleabi -meabi=4 -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-EABI
-; RUN: llc < %s -mtriple=arm-none-musleabihf -meabi=4 -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-EABI
+; RUN: llc < %s -mtriple=arm-none-musleabihf -mattr=+vfp2 -meabi=4 -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-EABI
 ; RUN: llc < %s -mtriple=arm-none-eabi -meabi=5 -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-EABI
-; RUN: llc < %s -mtriple=arm-none-eabihf -meabi=5 -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-EABI
+; RUN: llc < %s -mtriple=arm-none-eabihf -mattr=+vfp2 -meabi=5 -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-EABI
 ; RUN: llc < %s -mtriple=arm-none-androideabi -meabi=5 -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-EABI
 ; RUN: llc < %s -mtriple=arm-none-gnueabi -meabi=5 -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-EABI
-; RUN: llc < %s -mtriple=arm-none-gnueabihf -meabi=5 -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-EABI
+; RUN: llc < %s -mtriple=arm-none-gnueabihf -mattr=+vfp2 -meabi=5 -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-EABI
 ; RUN: llc < %s -mtriple=arm-none-musleabi -meabi=5 -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-EABI
-; RUN: llc < %s -mtriple=arm-none-musleabihf -meabi=5 -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-EABI
+; RUN: llc < %s -mtriple=arm-none-musleabihf -mattr=+vfp2 -meabi=5 -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-EABI
 
 %struct.my_s = type { [18 x i32] }
 
 define void @foo(i32* %t) {
   ; CHECK-LABEL: foo
 
   %1 = alloca i32*, align 4
   store i32* %t, i32** %1, align 4

test/CodeGen/ARM/fold-stack-adjust-2.ll

+; Disable shrink-wrapping on the first test otherwise we wouldn't
+; exerce the path for PR18136.
+; RUN: llc -mtriple=thumbv7-apple-none-macho < %s -enable-shrink-wrap=false | FileCheck %s
+
+
+declare void @bar(i8*)
+
+%bigVec = type [2 x double]
+
+@var = global %bigVec zeroinitializer
+
+define arm_aapcs_vfpcc double @check_vfp_no_return_clobber() minsize {
+; CHECK-LABEL: check_vfp_no_return_clobber:
+; CHECK: push {r[[GLOBREG:[0-9]+]], lr}
+; CHECK: vpush {d0, d1, d2, d3, d4, d5, d6, d7, d8, d9}
+; CHECK-NOT: sub sp,
+; ...
+; CHECK: add sp, #64
+; CHECK: vpop {d8, d9}
+; CHECK: pop {r[[GLOBREG]], pc}
+
+  %var = alloca i8, i32 64
+
+  %tmp = load %bigVec, %bigVec* @var
+  call void @bar(i8* %var)
+  store %bigVec %tmp, %bigVec* @var
+
+  ret double 1.0
+}

test/CodeGen/ARM/fold-stack-adjust.ll

; CHECK: pop {r7, pc}
   ; Just to keep iOS FileCheck within previous function:
 ; CHECK-IOS-LABEL: check_no_return_clobber:
 
   %var = alloca i8, i32 20
   call void @bar(i8* %var)
   ret i64 0
 }
 
-define arm_aapcs_vfpcc double @check_vfp_no_return_clobber() minsize {
-; CHECK-LABEL: check_vfp_no_return_clobber:
-; CHECK: push {r[[GLOBREG:[0-9]+]], lr}
-; CHECK: vpush {d0, d1, d2, d3, d4, d5, d6, d7, d8, d9}
-; CHECK-NOT: sub sp,
-; ...
-; CHECK: add sp, #64
-; CHECK: vpop {d8, d9}
-; CHECK: pop {r[[GLOBREG]], pc}
-
-  %var = alloca i8, i32 64
-
-  %tmp = load %bigVec, %bigVec* @var
-  call void @bar(i8* %var)
-  store %bigVec %tmp, %bigVec* @var
-
-  ret double 1.0
-}
-
 @dbl = global double 0.0
 
 ; PR18136: there was a bug determining where the first eligible pop in a
 ; basic-block was when the entire block was epilogue code.
 define void @test_fold_point(i1 %tst) minsize {
 ; CHECK-LABEL: test_fold_point:
 
   ; Important to check for beginning of basic block, because if it gets

test/CodeGen/ARM/fp16-promote.ll

-; RUN: llc -asm-verbose=false < %s -mattr=+vfp3,+fp16 | FileCheck %s -check-prefix=CHECK-FP16  --check-prefix=CHECK-VFP -check-prefix=CHECK-ALL
-; RUN: llc -asm-verbose=false < %s | FileCheck %s -check-prefix=CHECK-LIBCALL --check-prefix=CHECK-VFP -check-prefix=CHECK-ALL --check-prefix=CHECK-LIBCALL-VFP
-; RUN: llc -asm-verbose=false < %s -mattr=-vfp2 | FileCheck %s --check-prefix=CHECK-LIBCALL -check-prefix=CHECK-NOVFP -check-prefix=CHECK-ALL
+; RUN: llc -asm-verbose=false < %s -mtriple=armv7---eabihf -mattr=+vfp3,+fp16 | FileCheck %s -check-prefix=CHECK-FP16  --check-prefix=CHECK-VFP -check-prefix=CHECK-ALL
+; RUN: llc -asm-verbose=false < %s -mtriple=armv7---eabihf | FileCheck %s -check-prefix=CHECK-LIBCALL --check-prefix=CHECK-VFP -check-prefix=CHECK-ALL --check-prefix=CHECK-LIBCALL-VFP
+; RUN: llc -asm-verbose=false < %s -mattr=-vfp2 -mtriple=armv7---eabi | FileCheck %s --check-prefix=CHECK-LIBCALL -check-prefix=CHECK-NOVFP -check-prefix=CHECK-ALL
 
 target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-n32"
-target triple = "armv7---eabihf"
 
 ; CHECK-ALL-LABEL: test_fadd:
 ; CHECK-FP16: vcvtb.f32.f16
 ; CHECK-FP16: vcvtb.f32.f16
 ; CHECK-LIBCALL: bl __aeabi_h2f
 ; CHECK-LIBCALL: bl __aeabi_h2f
 ; CHECK-VFP: vadd.f32
 ; CHECK-NOVFP: bl __aeabi_fadd

test/CodeGen/ARM/memfunc.ll

 ; RUN: llc < %s -mtriple=armv7-apple-ios -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-IOS --check-prefix=CHECK
 ; RUN: llc < %s -mtriple=thumbv7m-none-macho -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-DARWIN --check-prefix=CHECK
 ; RUN: llc < %s -mtriple=arm-none-eabi -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-EABI --check-prefix=CHECK
-; RUN: llc < %s -mtriple=arm-none-eabihf -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-EABI --check-prefix=CHECK
+; RUN: llc < %s -mtriple=arm-none-eabihf -mattr=+vfp2 -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-EABI --check-prefix=CHECK
 ; RUN: llc < %s -mtriple=arm-none-androideabi -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-EABI --check-prefix=CHECK
 ; RUN: llc < %s -mtriple=arm-none-gnueabi -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-GNUEABI --check-prefix=CHECK
-; RUN: llc < %s -mtriple=arm-none-gnueabihf -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-GNUEABI --check-prefix=CHECK
+; RUN: llc < %s -mtriple=arm-none-gnueabihf -mattr=+vfp2 -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-GNUEABI --check-prefix=CHECK
 ; RUN: llc < %s -mtriple=arm-none-musleabi -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-GNUEABI --check-prefix=CHECK
-; RUN: llc < %s -mtriple=arm-none-musleabihf -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-GNUEABI --check-prefix=CHECK
+; RUN: llc < %s -mtriple=arm-none-musleabihf -mattr=+vfp2 -disable-post-ra -o - | FileCheck %s --check-prefix=CHECK-GNUEABI --check-prefix=CHECK
 
 define void @f1(i8* %dest, i8* %src) "no-frame-pointer-elim"="true" {
 entry:
   ; CHECK-LABEL: f1
 
   ; CHECK-IOS: bl _memmove
   ; CHECK-DARWIN: bl _memmove
   ; CHECK-EABI: bl __aeabi_memmove