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

[Hans Wennborg]

On Thu, Feb 12, 2015 at 3:35 PM, Hal Finkel <hfinkel at anl.gov> wrote:
> ----- 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.

Thanks for clarifying. Merged in r228986.

Cheers,
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