<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<meta name="Generator" content="Microsoft Exchange Server">
<!-- converted from text --><style><!-- .EmailQuote { margin-left: 1pt; padding-left: 4pt; border-left: #800000 2px solid; } --></style>
</head>
<body>
<meta content="text/html; charset=UTF-8">
<style type="text/css" style="">
<!--
p
        {margin-top:0;
        margin-bottom:0}
-->
</style>
<div dir="ltr">
<div id="x_divtagdefaultwrapper" style="font-size:12pt; color:#000000; background-color:#FFFFFF; font-family:Calibri,Arial,Helvetica,sans-serif">
<p>As far as can see this was a latent bug from around late 2015, but the PR was reported for trunk.</p>
<p>We could attempt running the regression test on 3.9, and if the bug is there it should trigger an assert.</p>
<p><br>
</p>
<p>-Silviu</p>
</div>
<hr tabindex="-1" style="display:inline-block; width:98%">
<div id="x_divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> hwennborg@google.com <hwennborg@google.com> on behalf of Hans Wennborg <hans@chromium.org><br>
<b>Sent:</b> 09 August 2016 21:43:02<br>
<b>To:</b> Silviu Baranga<br>
<b>Cc:</b> llvm-commits<br>
<b>Subject:</b> Re: [llvm] r278002 - [AArch64] PR28877: Don't assume we're running after legalization when creating vcvtfp2fxs</font>
<div> </div>
</div>
</div>
<font size="2"><span style="font-size:10pt;">
<div class="PlainText">Was this a bug affecting 3.9, or a more recent regression?<br>
<br>
On Mon, Aug 8, 2016 at 6:13 AM, Silviu Baranga via llvm-commits<br>
<llvm-commits@lists.llvm.org> wrote:<br>
> Author: sbaranga<br>
> Date: Mon Aug  8 08:13:57 2016<br>
> New Revision: 278002<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=278002&view=rev">http://llvm.org/viewvc/llvm-project?rev=278002&view=rev</a><br>
> Log:<br>
> [AArch64] PR28877: Don't assume we're running after legalization when creating vcvtfp2fxs<br>
><br>
> Summary:<br>
> The DAG combine transformation that was generating the<br>
> aarch64_neon_vcvtfp2fxs node was assuming that all<br>
> inputs where legal and wasn't accounting that the input<br>
> could be a v4f64 if we're trying to do the transformation<br>
> before legalization. We now bail out in this case.<br>
><br>
> All illegal types besides v4f64 were already rejected.<br>
><br>
> Fixes <a href="https://llvm.org/bugs/show_bug.cgi?id=28877">https://llvm.org/bugs/show_bug.cgi?id=28877</a>.<br>
><br>
> Reviewers: jmolloy<br>
><br>
> Subscribers: aemerson, rengolin, llvm-commits<br>
><br>
> Differential Revision: <a href="https://reviews.llvm.org/D23261">https://reviews.llvm.org/D23261</a><br>
><br>
> Added:<br>
>     llvm/trunk/test/CodeGen/AArch64/aarch64-vcvtfp2fxs-combine.ll<br>
> Modified:<br>
>     llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp<br>
><br>
> Modified: llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp?rev=278002&r1=278001&r2=278002&view=diff">
http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp?rev=278002&r1=278001&r2=278002&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp (original)<br>
> +++ llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp Mon Aug  8 08:13:57 2016<br>
> @@ -7684,6 +7684,7 @@ static SDValue performIntToFpCombine(SDN<br>
>  /// Fold a floating-point multiply by power of two into floating-point to<br>
>  /// fixed-point conversion.<br>
>  static SDValue performFpToIntCombine(SDNode *N, SelectionDAG &DAG,<br>
> +                                     TargetLowering::DAGCombinerInfo &DCI,<br>
>                                       const AArch64Subtarget *Subtarget) {<br>
>    if (!Subtarget->hasNEON())<br>
>      return SDValue();<br>
> @@ -7727,10 +7728,16 @@ static SDValue performFpToIntCombine(SDN<br>
>      ResTy = FloatBits == 32 ? MVT::v2i32 : MVT::v2i64;<br>
>      break;<br>
>    case 4:<br>
> -    ResTy = MVT::v4i32;<br>
> +    ResTy = FloatBits == 32 ? MVT::v4i32 : MVT::v4i64;<br>
>      break;<br>
>    }<br>
><br>
> +  if (ResTy == MVT::v4i64 && DCI.isBeforeLegalizeOps())<br>
> +    return SDValue();<br>
> +<br>
> +  assert((ResTy != MVT::v4i64 || DCI.isBeforeLegalizeOps()) &&<br>
> +         "Illegal vector type after legalization");<br>
> +<br>
>    SDLoc DL(N);<br>
>    bool IsSigned = N->getOpcode() == ISD::FP_TO_SINT;<br>
>    unsigned IntrinsicOpcode = IsSigned ? Intrinsic::aarch64_neon_vcvtfp2fxs<br>
> @@ -9852,7 +9859,7 @@ SDValue AArch64TargetLowering::PerformDA<br>
>      return performIntToFpCombine(N, DAG, Subtarget);<br>
>    case ISD::FP_TO_SINT:<br>
>    case ISD::FP_TO_UINT:<br>
> -    return performFpToIntCombine(N, DAG, Subtarget);<br>
> +    return performFpToIntCombine(N, DAG, DCI, Subtarget);<br>
>    case ISD::FDIV:<br>
>      return performFDivCombine(N, DAG, Subtarget);<br>
>    case ISD::OR:<br>
><br>
> Added: llvm/trunk/test/CodeGen/AArch64/aarch64-vcvtfp2fxs-combine.ll<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/aarch64-vcvtfp2fxs-combine.ll?rev=278002&view=auto">
http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/aarch64-vcvtfp2fxs-combine.ll?rev=278002&view=auto</a><br>
> ==============================================================================<br>
> --- llvm/trunk/test/CodeGen/AArch64/aarch64-vcvtfp2fxs-combine.ll (added)<br>
> +++ llvm/trunk/test/CodeGen/AArch64/aarch64-vcvtfp2fxs-combine.ll Mon Aug  8 08:13:57 2016<br>
> @@ -0,0 +1,24 @@<br>
> +; RUN: llc < %s -mtriple=aarch64-linux-eabi -o - | FileCheck %s<br>
> +<br>
> +%struct.a= type { i64, i64, i64, i64 }<br>
> +<br>
> +; DAG combine will try to perform a transformation that  creates a vcvtfp2fxs<br>
> +; with a v4f64 input. Since v4i64 is not legal we should bail out. We can<br>
> +; pottentially still create the vcvtfp2fxs node after legalization (but on a<br>
> +; v2f64).<br>
> +<br>
> +; CHECK-LABEL: fun1<br>
> +define void @fun1() local_unnamed_addr {<br>
> +entry:<br>
> +  %mul = fmul <4 x double> zeroinitializer, <double 6.553600e+04, double 6.553600e+04, double 6.553600e+04, double 6.553600e+04><br>
> +  %toi = fptosi <4 x double> %mul to <4 x i64><br>
> +  %ptr = getelementptr inbounds %struct.a, %struct.a* undef, i64 0, i32 2<br>
> +  %elem = extractelement <4 x i64> %toi, i32 1<br>
> +  store i64 %elem, i64* %ptr, align 8<br>
> +  call void @llvm.trap()<br>
> +  unreachable<br>
> +}<br>
> +<br>
> +; Function Attrs: noreturn nounwind<br>
> +declare void @llvm.trap()<br>
> +<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> llvm-commits@lists.llvm.org<br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
<br>
</div>
</span></font>
</body>
</html>