This is an archive of the discontinued LLVM Phabricator instance.

Differential D119428

[AArch64][SVE] Fix selection failure caused by fp/int convert using non-Neon types
ClosedPublic

Authored by bsmith on Feb 10 2022, 4:48 AM.

Details

Reviewers
paulwalker-arm
peterwaller-arm
sdesmalen
efriedma
Commits
rGc53ad72aa936: [AArch64][SVE] Fix selection failure caused by fp/int convert using non-Neon…
Summary

Fixes: #53679

Diff Detail

Event Timeline

bsmith created this revision.Feb 10 2022, 4:48 AM
Herald added a reviewer: efriedma. · View Herald TranscriptFeb 10 2022, 4:48 AM
Herald added subscribers: psnobl, hiraditya, kristof.beyls, tschuett. · View Herald Transcript
bsmith requested review of this revision.Feb 10 2022, 4:48 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 10 2022, 4:48 AM
Herald added a subscriber: llvm-commits. · View Herald Transcript
paulwalker-arm added inline comments.Feb 10 2022, 5:07 AM
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
13734

As is my usual comment. Can this test be tightened up?

rscottmanley added a subscriber: rscottmanley.Feb 10 2022, 5:51 AM
Harbormaster completed remote builds in B148709: Diff 407476.Feb 10 2022, 6:07 AM
bsmith updated this revision to Diff 407526.Feb 10 2022, 7:06 AM
  • Tighten check around type sizes
paulwalker-arm accepted this revision.Feb 10 2022, 7:12 AM
paulwalker-arm added inline comments.
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
13727–13728

I'm happy but for bonus points you should be able to remove this.

This revision is now accepted and ready to land.Feb 10 2022, 7:12 AM
Harbormaster completed remote builds in B148740: Diff 407526.Feb 10 2022, 7:50 AM
Closed by commit rGc53ad72aa936: [AArch64][SVE] Fix selection failure caused by fp/int convert using non-Neon… (authored by bsmith). · Explain WhyFeb 11 2022, 3:57 AM
This revision was automatically updated to reflect the committed changes.
bsmith added a commit: rGc53ad72aa936: [AArch64][SVE] Fix selection failure caused by fp/int convert using non-Neon….

Revision Contents

PathSize
llvm/
lib/
Target/
AArch64/
6 lines
test/
CodeGen/
AArch64/
25 lines

Diff 407834

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

  • This file is larger than 256 KB, so syntax highlighting is disabled by default.
Show First 20 Lines • Show All 13,718 Lines • ▼ Show 20 Linesstatic SDValue performFpToIntCombine(SDNode *N, SelectionDAG &DAG,
TargetLowering::DAGCombinerInfo &DCI, TargetLowering::DAGCombinerInfo &DCI,
const AArch64Subtarget *Subtarget) { const AArch64Subtarget *Subtarget) {
if (!Subtarget->hasNEON()) if (!Subtarget->hasNEON())
return SDValue(); return SDValue();
if (!N->getValueType(0).isSimple()) if (!N->getValueType(0).isSimple())
return SDValue(); return SDValue();
SDValue Op = N->getOperand(0); SDValue Op = N->getOperand(0);
if (!Op.getValueType().isVector() || !Op.getValueType().isSimple() || if (!Op.getValueType().isSimple() || Op.getOpcode() != ISD::FMUL)
paulwalker-armUnsubmitted
Not Done
ReplyInline Actions

I'm happy but for bonus points you should be able to remove this.

paulwalker-arm: I'm happy but for bonus points you should be able to remove this.
Op.getOpcode() != ISD::FMUL) return SDValue();
if (!Op.getValueType().is64BitVector() && !Op.getValueType().is128BitVector())
return SDValue(); return SDValue();
SDValue ConstVec = Op->getOperand(1); SDValue ConstVec = Op->getOperand(1);
paulwalker-armUnsubmitted
Not Done
ReplyInline Actions

As is my usual comment. Can this test be tightened up?

paulwalker-arm: As is my usual comment. Can this test be tightened up?
if (!isa<BuildVectorSDNode>(ConstVec)) if (!isa<BuildVectorSDNode>(ConstVec))
return SDValue(); return SDValue();
MVT FloatTy = Op.getSimpleValueType().getVectorElementType(); MVT FloatTy = Op.getSimpleValueType().getVectorElementType();
uint32_t FloatBits = FloatTy.getSizeInBits(); uint32_t FloatBits = FloatTy.getSizeInBits();
if (FloatBits != 32 && FloatBits != 64 && if (FloatBits != 32 && FloatBits != 64 &&
(FloatBits != 16 || !Subtarget->hasFullFP16())) (FloatBits != 16 || !Subtarget->hasFullFP16()))
return SDValue(); return SDValue();
▲ Show 20 Lines • Show All 6,603 Lines • Show Last 20 Lines

llvm/test/CodeGen/AArch64/sve-fixed-length-fp-convert.ll

  • This file was added.
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
; RUN: llc < %s | FileCheck %s
target triple = "aarch64-unknown-linux-gnu"
; Ensure we don't crash when trying to combine fp<->int conversions
define void @fp_convert_combine_crash(<8 x float> *%a, <8 x i32> *%b) #0 {
; CHECK-LABEL: fp_convert_combine_crash:
; CHECK: // %bb.0:
; CHECK-NEXT: ptrue p0.s
; CHECK-NEXT: fmov z1.s, #8.00000000
; CHECK-NEXT: ld1w { z0.s }, p0/z, [x0]
; CHECK-NEXT: fmul z0.s, z0.s, z1.s
; CHECK-NEXT: fcvtzs z0.s, p0/m, z0.s
; CHECK-NEXT: st1w { z0.s }, p0, [x1]
; CHECK-NEXT: ret
%f = load <8 x float>, <8 x float>* %a
%mul.i = fmul <8 x float> %f, <float 8.000000e+00, float 8.000000e+00, float 8.000000e+00, float 8.000000e+00,
float 8.000000e+00, float 8.000000e+00, float 8.000000e+00, float 8.000000e+00>
%vcvt.i = fptosi <8 x float> %mul.i to <8 x i32>
store <8 x i32> %vcvt.i, <8 x i32>* %b
ret void
}
attributes #0 = { vscale_range(2,2) "target-features"="+sve" }