[llvm] r228969 - [SDAG] Don't try to use FP_EXTEND/FP_ROUND for int<->fp promotions

[Hal Finkel]

----- Original Message -----
> From: "Hans Wennborg" <hans at chromium.org>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: "llvm-commits" <llvm-commits at cs.uiuc.edu>, "Owen Anderson" <owen at apple.com>
> Sent: Thursday, February 12, 2015 5:31:09 PM
> Subject: Re: [llvm] r228969 - [SDAG] Don't try to use FP_EXTEND/FP_ROUND for int<->fp promotions
> 
> Is it a correctness or performance regression? If it's the latter,
> I'd
> be hesitant to merge it at this stage.

No, this is a correctness issue (DAG.getNode(ISD::FP_EXTEND, ...) will assert if you give it an integer data type). Regarding performance issues, I certainly agree.

 -Hal

> 
>  - Hans
> 
> On Thu, Feb 12, 2015 at 3:16 PM, Hal Finkel <hfinkel at anl.gov> wrote:
> > Owen, Hans,
> >
> > This fixes a regression from 3.5, affecting PowerPC (and perhaps
> > also other out-of-tree targets). Could we bring it into the 3.6
> > release branch?
> >
> > Unfortunately, I missed this for a long time because of an
> > oversight on my part in how I was running the test suite. :(
> >
> > Thanks again,
> > Hal
> >
> > ----- Original Message -----
> >> From: "Hal Finkel" <hfinkel at anl.gov>
> >> To: llvm-commits at cs.uiuc.edu
> >> Sent: Thursday, February 12, 2015 4:43:52 PM
> >> Subject: [llvm] r228969 - [SDAG] Don't try to use
> >> FP_EXTEND/FP_ROUND for      int<->fp promotions
> >>
> >> Author: hfinkel
> >> Date: Thu Feb 12 16:43:52 2015
> >> New Revision: 228969
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=228969&view=rev
> >> Log:
> >> [SDAG] Don't try to use FP_EXTEND/FP_ROUND for int<->fp promotions
> >>
> >> The PowerPC backend has long promoted some floating-point vector
> >> operations
> >> (such as select) to integer vector operations. Unfortunately, this
> >> behavior was
> >> broken by r216555. When using FP_EXTEND/FP_ROUND for promotions,
> >> we
> >> must check
> >> that both the old and new types are floating-point types.
> >> Otherwise,
> >> we must
> >> use BITCAST as we did prior to r216555 for everything.
> >>
> >> Added:
> >>     llvm/trunk/test/CodeGen/PowerPC/vsel-prom.ll
> >> Modified:
> >>     llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp
> >>
> >> Modified:
> >> llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp
> >> URL:
> >> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp?rev=228969&r1=228968&r2=228969&view=diff
> >> ==============================================================================
> >> --- llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp
> >> (original)
> >> +++ llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp Thu
> >> Feb
> >> 12 16:43:52 2015
> >> @@ -390,7 +390,8 @@ SDValue VectorLegalizer::Promote(SDValue
> >>        if (Op.getOperand(j)
> >>                .getValueType()
> >>                .getVectorElementType()
> >> -              .isFloatingPoint())
> >> +              .isFloatingPoint() &&
> >> +          NVT.isVector() &&
> >> NVT.getVectorElementType().isFloatingPoint())
> >>          Operands[j] = DAG.getNode(ISD::FP_EXTEND, dl, NVT,
> >>          Op.getOperand(j));
> >>        else
> >>          Operands[j] = DAG.getNode(ISD::BITCAST, dl, NVT,
> >>          Op.getOperand(j));
> >> @@ -399,8 +400,9 @@ SDValue VectorLegalizer::Promote(SDValue
> >>    }
> >>
> >>    Op = DAG.getNode(Op.getOpcode(), dl, NVT, Operands);
> >> -  if (VT.isFloatingPoint() ||
> >> -      (VT.isVector() &&
> >> VT.getVectorElementType().isFloatingPoint()))
> >> +  if ((VT.isFloatingPoint() && NVT.isFloatingPoint()) ||
> >> +      (VT.isVector() &&
> >> VT.getVectorElementType().isFloatingPoint()
> >> &&
> >> +       NVT.isVector() &&
> >> NVT.getVectorElementType().isFloatingPoint()))
> >>      return DAG.getNode(ISD::FP_ROUND, dl, VT, Op,
> >>      DAG.getIntPtrConstant(0));
> >>    else
> >>      return DAG.getNode(ISD::BITCAST, dl, VT, Op);
> >>
> >> Added: llvm/trunk/test/CodeGen/PowerPC/vsel-prom.ll
> >> URL:
> >> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/PowerPC/vsel-prom.ll?rev=228969&view=auto
> >> ==============================================================================
> >> --- llvm/trunk/test/CodeGen/PowerPC/vsel-prom.ll (added)
> >> +++ llvm/trunk/test/CodeGen/PowerPC/vsel-prom.ll Thu Feb 12
> >> 16:43:52
> >> 2015
> >> @@ -0,0 +1,23 @@
> >> +; RUN: llc -mcpu=pwr7 < %s | FileCheck %s
> >> +target datalayout = "E-m:e-i64:64-n32:64"
> >> +target triple = "powerpc64-unknown-linux-gnu"
> >> +
> >> +; Function Attrs: nounwind
> >> +define void @Compute_Lateral() #0 {
> >> +entry:
> >> +  br i1 undef, label %if.then, label %if.end
> >> +
> >> +if.then:                                          ; preds =
> >> %entry
> >> +  unreachable
> >> +
> >> +if.end:                                           ; preds =
> >> %entry
> >> +  %0 = select i1 undef, <2 x double> undef, <2 x double>
> >> zeroinitializer
> >> +  %1 = extractelement <2 x double> %0, i32 1
> >> +  store double %1, double* undef, align 8
> >> +  ret void
> >> +
> >> +; CHECK-LABEL: @Compute_Lateral
> >> +}
> >> +
> >> +attributes #0 = { nounwind }
> >> +
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at cs.uiuc.edu
> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >>
> >
> > --
> > Hal Finkel
> > Assistant Computational Scientist
> > Leadership Computing Facility
> > Argonne National Laboratory
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory