[llvm] r278002 - [AArch64] PR28877: Don't assume we're running after legalization when creating vcvtfp2fxs

[Silviu Baranga via llvm-commits]

As far as can see this was a latent bug from around late 2015, but the PR was reported for trunk.

We could attempt running the regression test on 3.9, and if the bug is there it should trigger an assert.


-Silviu

________________________________
From: hwennborg at google.com <hwennborg at google.com> on behalf of Hans Wennborg <hans at chromium.org>
Sent: 09 August 2016 21:43:02
To: Silviu Baranga
Cc: llvm-commits
Subject: Re: [llvm] r278002 - [AArch64] PR28877: Don't assume we're running after legalization when creating vcvtfp2fxs

Was this a bug affecting 3.9, or a more recent regression?

On Mon, Aug 8, 2016 at 6:13 AM, Silviu Baranga via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Author: sbaranga
> Date: Mon Aug  8 08:13:57 2016
> New Revision: 278002
>
> URL: http://llvm.org/viewvc/llvm-project?rev=278002&view=rev
> Log:
> [AArch64] PR28877: Don't assume we're running after legalization when creating vcvtfp2fxs
>
> Summary:
> The DAG combine transformation that was generating the
> aarch64_neon_vcvtfp2fxs node was assuming that all
> inputs where legal and wasn't accounting that the input
> could be a v4f64 if we're trying to do the transformation
> before legalization. We now bail out in this case.
>
> All illegal types besides v4f64 were already rejected.
>
> Fixes https://llvm.org/bugs/show_bug.cgi?id=28877.
>
> Reviewers: jmolloy
>
> Subscribers: aemerson, rengolin, llvm-commits
>
> Differential Revision: https://reviews.llvm.org/D23261
>
> Added:
>     llvm/trunk/test/CodeGen/AArch64/aarch64-vcvtfp2fxs-combine.ll
> Modified:
>     llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp
>
> Modified: llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp?rev=278002&r1=278001&r2=278002&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp (original)
> +++ llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp Mon Aug  8 08:13:57 2016
> @@ -7684,6 +7684,7 @@ static SDValue performIntToFpCombine(SDN
>  /// Fold a floating-point multiply by power of two into floating-point to
>  /// fixed-point conversion.
>  static SDValue performFpToIntCombine(SDNode *N, SelectionDAG &DAG,
> +                                     TargetLowering::DAGCombinerInfo &DCI,
>                                       const AArch64Subtarget *Subtarget) {
>    if (!Subtarget->hasNEON())
>      return SDValue();
> @@ -7727,10 +7728,16 @@ static SDValue performFpToIntCombine(SDN
>      ResTy = FloatBits == 32 ? MVT::v2i32 : MVT::v2i64;
>      break;
>    case 4:
> -    ResTy = MVT::v4i32;
> +    ResTy = FloatBits == 32 ? MVT::v4i32 : MVT::v4i64;
>      break;
>    }
>
> +  if (ResTy == MVT::v4i64 && DCI.isBeforeLegalizeOps())
> +    return SDValue();
> +
> +  assert((ResTy != MVT::v4i64 || DCI.isBeforeLegalizeOps()) &&
> +         "Illegal vector type after legalization");
> +
>    SDLoc DL(N);
>    bool IsSigned = N->getOpcode() == ISD::FP_TO_SINT;
>    unsigned IntrinsicOpcode = IsSigned ? Intrinsic::aarch64_neon_vcvtfp2fxs
> @@ -9852,7 +9859,7 @@ SDValue AArch64TargetLowering::PerformDA
>      return performIntToFpCombine(N, DAG, Subtarget);
>    case ISD::FP_TO_SINT:
>    case ISD::FP_TO_UINT:
> -    return performFpToIntCombine(N, DAG, Subtarget);
> +    return performFpToIntCombine(N, DAG, DCI, Subtarget);
>    case ISD::FDIV:
>      return performFDivCombine(N, DAG, Subtarget);
>    case ISD::OR:
>
> Added: llvm/trunk/test/CodeGen/AArch64/aarch64-vcvtfp2fxs-combine.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/aarch64-vcvtfp2fxs-combine.ll?rev=278002&view=auto
> ==============================================================================
> --- llvm/trunk/test/CodeGen/AArch64/aarch64-vcvtfp2fxs-combine.ll (added)
> +++ llvm/trunk/test/CodeGen/AArch64/aarch64-vcvtfp2fxs-combine.ll Mon Aug  8 08:13:57 2016
> @@ -0,0 +1,24 @@
> +; RUN: llc < %s -mtriple=aarch64-linux-eabi -o - | FileCheck %s
> +
> +%struct.a= type { i64, i64, i64, i64 }
> +
> +; DAG combine will try to perform a transformation that  creates a vcvtfp2fxs
> +; with a v4f64 input. Since v4i64 is not legal we should bail out. We can
> +; pottentially still create the vcvtfp2fxs node after legalization (but on a
> +; v2f64).
> +
> +; CHECK-LABEL: fun1
> +define void @fun1() local_unnamed_addr {
> +entry:
> +  %mul = fmul <4 x double> zeroinitializer, <double 6.553600e+04, double 6.553600e+04, double 6.553600e+04, double 6.553600e+04>
> +  %toi = fptosi <4 x double> %mul to <4 x i64>
> +  %ptr = getelementptr inbounds %struct.a, %struct.a* undef, i64 0, i32 2
> +  %elem = extractelement <4 x i64> %toi, i32 1
> +  store i64 %elem, i64* %ptr, align 8
> +  call void @llvm.trap()
> +  unreachable
> +}
> +
> +; Function Attrs: noreturn nounwind
> +declare void @llvm.trap()
> +
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160809/770f6907/attachment.html>