`@llvm.abs.i64` optimizes differently versus target-specific `@llvm.aarch64.neon.abs.i64`

[folkertdev]

I'm not sure one is really better than the other, but it's the difference that is weird. My reading of https://llvm.org/docs/LangRef.html#llvm-abs-intrinsic is that the second argument being false means that the behavior is in fact wrapping, like the neon instruction.

https://godbolt.org/z/fTvxc4z3z

target triple = "aarch64-unknown-linux-gnu"

define noundef i64 @foo(i64 noundef %a) unnamed_addr {
start:
  %_0.sroa.0.0 = tail call i64 @llvm.abs.i64(i64 %a, i1 false)
  ret i64 %_0.sroa.0.0
}

define noundef i64 @bar(i64 noundef %a) unnamed_addr {
start:
  %_0.i = tail call noundef i64 @llvm.aarch64.neon.abs.i64(i64 noundef %a) #3
  ret i64 %_0.i
}

declare i64 @llvm.aarch64.neon.abs.i64(i64) unnamed_addr #1

declare i64 @llvm.abs.i64(i64, i1 immarg) #2

At -O0 they produce the same instructions, but at -O3 they do not

foo:                                    // @foo
        cmp     x0, #0
        cneg    x0, x0, mi
        ret
bar:                                    // @bar
        fmov    d0, x0
        abs     d0, d0
        fmov    x0, d0
        ret

[llvmbot]

@llvm/issue-subscribers-backend-aarch64

Author: Folkert de Vries (folkertdev)

I'm not sure one is really better than the other, but it's the difference that is weird. My reading of https://llvm.org/docs/LangRef.html#llvm-abs-intrinsic is that the second argument being false means that the behavior is in fact wrapping, like the neon instruction.

https://godbolt.org/z/fTvxc4z3z

target triple = "aarch64-unknown-linux-gnu"

define noundef i64 @<!-- -->foo(i64 noundef %a) unnamed_addr {
start:
  %_0.sroa.0.0 = tail call i64 @<!-- -->llvm.abs.i64(i64 %a, i1 false)
  ret i64 %_0.sroa.0.0
}

define noundef i64 @<!-- -->bar(i64 noundef %a) unnamed_addr {
start:
  %_0.i = tail call noundef i64 @<!-- -->llvm.aarch64.neon.abs.i64(i64 noundef %a) #<!-- -->3
  ret i64 %_0.i
}

declare i64 @<!-- -->llvm.aarch64.neon.abs.i64(i64) unnamed_addr #<!-- -->1

declare i64 @<!-- -->llvm.abs.i64(i64, i1 immarg) #<!-- -->2

At -O0 they produce the same instructions, but at -O3 they do not

foo:                                    // @<!-- -->foo
        cmp     x0, #<!-- -->0
        cneg    x0, x0, mi
        ret
bar:                                    // @<!-- -->bar
        fmov    d0, x0
        abs     d0, d0
        fmov    x0, d0
        ret

[folkertdev]

I suspect the code responsible for this difference is

https://github.com/llvm/llvm-project/blob/43a9ec2ecdf4aac8966e3b02e4e92c794f5b6a27/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp#L5858-L5867

  case Intrinsic::aarch64_neon_abs: {
    EVT Ty = Op.getValueType();
    if (Ty == MVT::i64) {
      SDValue Result = DAG.getNode(ISD::BITCAST, dl, MVT::v1i64,
                                   Op.getOperand(1));
      Result = DAG.getNode(ISD::ABS, dl, MVT::v1i64, Result);
      return DAG.getNode(ISD::BITCAST, dl, MVT::i64, Result);
    } else if (Ty.isVector() && Ty.isInteger() && isTypeLegal(Ty)) {
      return DAG.getNode(ISD::ABS, dl, Ty, Op.getOperand(1));
    } else {
      report_fatal_error("Unexpected type for AArch64 NEON intrinsic");
    }
  }

This casts the i64 to a v1i64 (so a 1-element SIMD vector) and then lowers to the abs implementation for that.

So, that seems deliberate, but also wrong. If anything, the 1-element simd vector should probably just use the scalar code path. Am I missing something here?

[davemgreen]

llvm.aarch64.neon.abs.i64 is only produced from vabsd_s64 intrinsics (https://developer.arm.com/architectures/instruction-sets/intrinsics/#q=vabsd_s64), so is a hint to use vector instructions for the abs operation. Using either GPRs or vector instructions would be valid.

It just uses abs+fmovs on d regs at the moment because they do not come up very often. GISel (-O0) will just treat single-element vectors as scalars, dropping the vector component (so would be less efficient the other way araound).

[folkertdev]

It's still not entirely clear to me which of these is better: is there an advantage to using vector instructions for this operation?

[davemgreen]

It will depend on where the data is already kept. If the value is already in a neon register then the neon variant is better. Otherwise the scalar version is fine.

(There is a scalar abs instruction added in a later architecture version, but that requires cssc which is a armv8.9 / armv9.4 feature).

[folkertdev]

Ok, that makes sense. Thanks for the background!