[llvm] r363998 - [Reassociate] Remove bogus assert reported in PR42349.

[Quentin Colombet via llvm-commits]

Hi Cameron,

> On Jun 20, 2019, at 4:03 PM, Cameron McInally via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Author: mcinally
> Date: Thu Jun 20 16:03:55 2019
> New Revision: 363998
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=363998&view=rev
> Log:
> [Reassociate] Remove bogus assert reported in PR42349.
> 
> Also, add a FIXME for the unsafe transform on a unary FNeg. A unary FNeg can only be transformed to a FMul by -1.0 when the nnan flag is present. The unary FNeg project is a WIP, so the unsafe transformation is acceptable until that work is complete.
> 
> The bogus assert with introduced in D63445.
> 
> Added:
>    llvm/trunk/test/Transforms/Reassociate/pr42349.ll
> Modified:
>    llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp
> 
> Modified: llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp?rev=363998&r1=363997&r2=363998&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp Thu Jun 20 16:03:55 2019
> @@ -268,11 +268,7 @@ static BinaryOperator *CreateNeg(Value *
> static BinaryOperator *LowerNegateToMultiply(Instruction *Neg) {
>   assert((isa<UnaryOperator>(Neg) || isa<BinaryOperator>(Neg)) &&
>          "Expected a Negate!");
> -  // It's not safe to lower a unary FNeg into a FMul by -1.0. However,
> -  // we can only reach this function with fast flags set, so it's
> -  // safe to do with nnan.
> -  assert((!isa<FPMathOperator>(Neg) || Neg->isFast()) &&
> -          "Expecting FastMathFlags!");
> +  // FIXME: It's not safe to lower a unary FNeg into a FMul by -1.0.
>   unsigned OpNo = isa<BinaryOperator>(Neg) ? 1 : 0;
>   Type *Ty = Neg->getType();
>   Constant *NegOne = Ty->isIntOrIntVectorTy() ?
> 
> Added: llvm/trunk/test/Transforms/Reassociate/pr42349.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Reassociate/pr42349.ll?rev=363998&view=auto
> ==============================================================================
> --- llvm/trunk/test/Transforms/Reassociate/pr42349.ll (added)
> +++ llvm/trunk/test/Transforms/Reassociate/pr42349.ll Thu Jun 20 16:03:55 2019
> @@ -0,0 +1,18 @@
> +; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
> +; RUN: opt < %s -reassociate -S | FileCheck %s
> +
> +define  float @wibble(float %tmp6) #0 {
> +; CHECK-LABEL: @wibble(
> +; CHECK-NEXT:  bb:
> +; CHECK-NEXT:    [[TMP7:%.*]] = fmul float [[TMP6:%.*]], -1.000000e+00
> +; CHECK-NEXT:    [[TMP0:%.*]] = fsub float -0.000000e+00, 0.000000e+00
> +; CHECK-NEXT:    [[TMP9:%.*]] = fmul fast float [[TMP6]], 0xFFF0000000000000
> +; CHECK-NEXT:    ret float [[TMP9]]

The IR looks wrong to me in that sample:
1. Both tmp0 and tmp7 are dead
2. tmp9 went from `(-%tmp6) * cst` to `%tmp6 * cst`, i.e., we dropped the fneg.

> +;
> +bb:
> +  %tmp7 = fsub float -0.000000e+00, %tmp6
> +  %tmp9 = fmul fast float %tmp7, 0x7FF0000000000000
> +  ret float %tmp9
> +}
> +
> +attributes #0 = { "use-soft-float"="false" }
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits