[Aarch64] Customer lowering of COPYSIGN to SIMD should check for NEON availability

[akshaykumars614]

Summary

For the following test case, clang is crashing for ARM64 architecture
$ cat crash.c
double crash(double a, double b)
{
return __builtin_copysign(a, b);
}

$ clang -O2 -march=armv8-a+nosimd --target=arm64 -S crash.c -o /dev/null
fatal error: error in backend: Cannot select: 0x7fae361bb4e8: v2i64 = AArch64ISD::BIT 0x7fae361bb210, 0x7fae361bb278, 0x7fae361bb480
Fix: PR51806

[dmgreen]

Sounds OK. AArch64 without NEON is not very well supported - it's presumed to be available on all implementations. There may be other bugs too, but we can fix the places we know about.

Can you add a test?

[akshaykumars614]

Updated the testcase, and formatted the file accordingly.

[dmgreen]

llvm/test/CodeGen/AArch64/fcopysign.ll

2–3

Can you make sure there is a "with neon" and a "without neon" run line. That way we test both cases.

The update script doesn't remove old CHECK lines that have been removed fro the RUN lines - those need to be removed manually (or keep the old run lines with the same check-prefix, they should then be updated).

16

This file only seems to contain f128 copysigns. Can we make sure there are f64/f32/f16 tests too?

[akshaykumars614]

@dmgreen , I have updated the run lines.
I am feeling difficult in finding example for adding f64/f32/f16 tests in the testcase. It will be helpful if you provide me with the example.

[dmgreen]

In D116581#3230813, @akshaykumars614 wrote:

@dmgreen , I have updated the run lines.
I am feeling difficult in finding example for adding f64/f32/f16 tests in the testcase. It will be helpful if you provide me with the example.

They should look something like this: https://godbolt.org/z/qvGs1G53M. With other half/double versions too.

llvm/test/CodeGen/AArch64/fcopysign.ll

2–3

We likely don't need the duplicate first line. I would change these run lines to:

; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
; RUN: llc < %s -march=aarch64-none-eabi | FileCheck %s
; RUN: llc < %s -march=aarch64-none-eabi -mattr -neon | FileCheck -check-prefix=CHECK-NONEON %s

And remove the "target triple = .." line below.

[akshaykumars614]

I have updated the testcase and changed the run lines.

[dmgreen]

llvm/test/CodeGen/AArch64/fcopysign.ll

8–10

We'll need to change the types for each of these too:

declare double @llvm.copysign.f64(double %a, double %b)
declare float @llvm.copysign.f32(float %a, float %b)
declare half @llvm.copysign.f16(half %a, half %b)

Then change the types below too.

[akshaykumars614]

I have updated the types as suggested.

[dmgreen]

llvm/test/CodeGen/AArch64/fcopysign.ll

8

These intrinsics in llvm are overloaded - the types they operate on are specified in the name of the intrinsic, and the type they operator on should match the name of the intrinsic (where float=f32, half=f16, etc). There are some details in https://llvm.org/docs/LangRef.html#intrinsic-functions

So it should be float @llvm.copysign.f32(float %a, float %b), double @llvm.copysign.f64(double %a, double %b) and half @llvm.copysign.f16(half %a, half %b) but not any of the other combinations here.

[akshaykumars614]

updated the changes suggested

[dmgreen]

Thanks. LGTM

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

if (VT.isScalableVector()) {
     SDValue Sign = DAG.getNode(ISD::AND, DL, IntVT, SignMask,
                                getSVESafeBitCast(IntVT, In2, DAG));
     SDValue Magnitude = DAG.getNode(ISD::AND, DL, IntVT, InvSignMask,
                                     getSVESafeBitCast(IntVT, In1, DAG));
     SDValue IntResult = DAG.getNode(ISD::OR, DL, IntVT, Sign, Magnitude);
     return getSVESafeBitCast(VT, IntResult, DAG);
   }
 
+  if (!Subtarget->hasNEON())
+    return SDValue();
+
   if (SrcVT.bitsLT(VT))
     In2 = DAG.getNode(ISD::FP_EXTEND, DL, VT, In2);
   else if (SrcVT.bitsGT(VT))
     In2 = DAG.getNode(ISD::FP_ROUND, DL, VT, In2, DAG.getIntPtrConstant(0, DL));
 
   EVT VecVT;
   uint64_t EltMask;
   SDValue VecVal1, VecVal2;

llvm/test/CodeGen/AArch64/fcopysign.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc -o - %s | FileCheck %s
+; RUN: llc < %s -mtriple=aarch64-none-eabi | FileCheck %s
+; RUN: llc < %s -mtriple=aarch64-none-eabi -mattr=-neon | FileCheck -check-prefix=CHECK-NONEON %s
 ; Check that selection dag legalization of fcopysign works in cases with
 ; different modes for the arguments.
-target triple = "aarch64--"
 
 declare fp128 @llvm.copysign.f128(fp128, fp128)
+declare float @llvm.copysign.f32(float %a, float %b)
+declare double @llvm.copysign.f64(double %a, double %b)
+declare half @llvm.copysign.f16(half %a, half %b)
 
 @val_float = dso_local global float zeroinitializer, align 4
 @val_double = dso_local global double zeroinitializer, align 8
 @val_fp128 = dso_local global fp128 zeroinitializer, align 16
 
 define fp128 @copysign0() {
 ; CHECK-LABEL: copysign0:
 ; CHECK:       // %bb.0: // %entry
 ; CHECK-NEXT:    .cfi_def_cfa_offset 16
 ; CHECK-NEXT:    adrp x8, .LCPI0_0
 ; CHECK-NEXT:    ldr q0, [x8, :lo12:.LCPI0_0]
 ; CHECK-NEXT:    adrp x8, val_double
 ; CHECK-NEXT:    str q0, [sp, #-16]!
 ; CHECK-NEXT:    ldr x8, [x8, :lo12:val_double]
 ; CHECK-NEXT:    ldrb w9, [sp, #15]
 ; CHECK-NEXT:    and x8, x8, #0x8000000000000000
 ; CHECK-NEXT:    lsr x8, x8, #56
 ; CHECK-NEXT:    bfxil w8, w9, #0, #7
 ; CHECK-NEXT:    strb w8, [sp, #15]
 ; CHECK-NEXT:    ldr q0, [sp], #16
 ; CHECK-NEXT:    ret
+;
+; CHECK-NONEON-LABEL: copysign0:
+; CHECK-NONEON:       // %bb.0: // %entry
+; CHECK-NONEON-NEXT:    .cfi_def_cfa_offset 16
+; CHECK-NONEON-NEXT:    adrp x8, .LCPI0_0
+; CHECK-NONEON-NEXT:    ldr q0, [x8, :lo12:.LCPI0_0]
+; CHECK-NONEON-NEXT:    adrp x8, val_double
+; CHECK-NONEON-NEXT:    str q0, [sp, #-16]!
+; CHECK-NONEON-NEXT:    ldr x8, [x8, :lo12:val_double]
+; CHECK-NONEON-NEXT:    ldrb w9, [sp, #15]
+; CHECK-NONEON-NEXT:    and x8, x8, #0x8000000000000000
+; CHECK-NONEON-NEXT:    lsr x8, x8, #56
+; CHECK-NONEON-NEXT:    bfxil w8, w9, #0, #7
+; CHECK-NONEON-NEXT:    strb w8, [sp, #15]
+; CHECK-NONEON-NEXT:    ldr q0, [sp], #16
+; CHECK-NONEON-NEXT:    ret
 entry:
   %v = load double, double* @val_double, align 8
   %conv = fpext double %v to fp128
   %call = tail call fp128 @llvm.copysign.f128(fp128 0xL00000000000000007FFF000000000000, fp128 %conv) #2
   ret fp128 %call
 }
 
 define fp128@copysign1() {
 ; CHECK-LABEL: copysign1:
 ; CHECK:       // %bb.0: // %entry
 ; CHECK-NEXT:    .cfi_def_cfa_offset 16
 ; CHECK-NEXT:    adrp x8, val_fp128
 ; CHECK-NEXT:    ldr q0, [x8, :lo12:val_fp128]
 ; CHECK-NEXT:    adrp x8, val_float
 ; CHECK-NEXT:    str q0, [sp, #-16]!
 ; CHECK-NEXT:    ldr w8, [x8, :lo12:val_float]
 ; CHECK-NEXT:    ldrb w9, [sp, #15]
 ; CHECK-NEXT:    and w8, w8, #0x80000000
 ; CHECK-NEXT:    lsr w8, w8, #24
 ; CHECK-NEXT:    bfxil w8, w9, #0, #7
 ; CHECK-NEXT:    strb w8, [sp, #15]
 ; CHECK-NEXT:    ldr q0, [sp], #16
 ; CHECK-NEXT:    ret
+;
+; CHECK-NONEON-LABEL: copysign1:
+; CHECK-NONEON:       // %bb.0: // %entry
+; CHECK-NONEON-NEXT:    .cfi_def_cfa_offset 16
+; CHECK-NONEON-NEXT:    adrp x8, val_fp128
+; CHECK-NONEON-NEXT:    ldr q0, [x8, :lo12:val_fp128]
+; CHECK-NONEON-NEXT:    adrp x8, val_float
+; CHECK-NONEON-NEXT:    str q0, [sp, #-16]!
+; CHECK-NONEON-NEXT:    ldr w8, [x8, :lo12:val_float]
+; CHECK-NONEON-NEXT:    ldrb w9, [sp, #15]
+; CHECK-NONEON-NEXT:    and w8, w8, #0x80000000
+; CHECK-NONEON-NEXT:    lsr w8, w8, #24
+; CHECK-NONEON-NEXT:    bfxil w8, w9, #0, #7
+; CHECK-NONEON-NEXT:    strb w8, [sp, #15]
+; CHECK-NONEON-NEXT:    ldr q0, [sp], #16
+; CHECK-NONEON-NEXT:    ret
 entry:
   %v0 = load fp128, fp128* @val_fp128, align 16
   %v1 = load float, float* @val_float, align 4
   %conv = fpext float %v1 to fp128
   %call = tail call fp128 @llvm.copysign.f128(fp128 %v0, fp128 %conv)
   ret fp128 %call
 }
+
+define float @copysign32(float %a, float %b) {
+; CHECK-LABEL: copysign32:
+; CHECK:       // %bb.0: // %entry
+; CHECK-NEXT:    movi v2.4s, #128, lsl #24
+; CHECK-NEXT:    // kill: def $s0 killed $s0 def $q0
+; CHECK-NEXT:    // kill: def $s1 killed $s1 def $q1
+; CHECK-NEXT:    bit v0.16b, v1.16b, v2.16b
+; CHECK-NEXT:    // kill: def $s0 killed $s0 killed $q0
+; CHECK-NEXT:    ret
+;
+; CHECK-NONEON-LABEL: copysign32:
+; CHECK-NONEON:       // %bb.0: // %entry
+; CHECK-NONEON-NEXT:    fabs s0, s0
+; CHECK-NONEON-NEXT:    fmov w8, s1
+; CHECK-NONEON-NEXT:    tst w8, #0x80000000
+; CHECK-NONEON-NEXT:    fneg s2, s0
+; CHECK-NONEON-NEXT:    fcsel s0, s2, s0, ne
+; CHECK-NONEON-NEXT:    ret
+entry:
+  %c = call float @llvm.copysign.f32(float %a, float %b)
+  ret float %c
+}
+
+define double @copysign64(double %a, double %b) {
+; CHECK-LABEL: copysign64:
+; CHECK:       // %bb.0: // %entry
+; CHECK-NEXT:    movi v2.2d, #0000000000000000
+; CHECK-NEXT:    // kill: def $d0 killed $d0 def $q0
+; CHECK-NEXT:    // kill: def $d1 killed $d1 def $q1
+; CHECK-NEXT:    fneg v2.2d, v2.2d
+; CHECK-NEXT:    bit v0.16b, v1.16b, v2.16b
+; CHECK-NEXT:    // kill: def $d0 killed $d0 killed $q0
+; CHECK-NEXT:    ret
+;
+; CHECK-NONEON-LABEL: copysign64:
+; CHECK-NONEON:       // %bb.0: // %entry
+; CHECK-NONEON-NEXT:    fabs d0, d0
+; CHECK-NONEON-NEXT:    fmov x8, d1
+; CHECK-NONEON-NEXT:    tst x8, #0x8000000000000000
+; CHECK-NONEON-NEXT:    fneg d2, d0
+; CHECK-NONEON-NEXT:    fcsel d0, d2, d0, ne
+; CHECK-NONEON-NEXT:    ret
+entry:
+  %c = call double @llvm.copysign.f64(double %a, double %b)
+  ret double %c
+}
+
+define half @copysign16(half %a, half %b) {
+; CHECK-LABEL: copysign16:
+; CHECK:       // %bb.0: // %entry
+; CHECK-NEXT:    movi v2.4s, #128, lsl #24
+; CHECK-NEXT:    fcvt s1, h1
+; CHECK-NEXT:    fcvt s0, h0
+; CHECK-NEXT:    bit v0.16b, v1.16b, v2.16b
+; CHECK-NEXT:    fcvt h0, s0
+; CHECK-NEXT:    ret
+;
+; CHECK-NONEON-LABEL: copysign16:
+; CHECK-NONEON:       // %bb.0: // %entry
+; CHECK-NONEON-NEXT:    sub sp, sp, #16
+; CHECK-NONEON-NEXT:    .cfi_def_cfa_offset 16
+; CHECK-NONEON-NEXT:    fcvt s0, h0
+; CHECK-NONEON-NEXT:    str h1, [sp, #12]
+; CHECK-NONEON-NEXT:    ldrb w8, [sp, #13]
+; CHECK-NONEON-NEXT:    fabs s0, s0
+; CHECK-NONEON-NEXT:    tst w8, #0x80
+; CHECK-NONEON-NEXT:    fneg s1, s0
+; CHECK-NONEON-NEXT:    fcsel s0, s1, s0, ne
+; CHECK-NONEON-NEXT:    fcvt h0, s0
+; CHECK-NONEON-NEXT:    add sp, sp, #16
+; CHECK-NONEON-NEXT:    ret
+entry:
+  %c = call half @llvm.copysign.f16(half %a, half %b)
+  ret half %c
+}