[llvm] r343727 - [InstCombine] allow SimplifyDemandedVectorElts to work with FP binops

via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 5 18:16:58 PDT 2018


Hi Sanjay,

This change caused a regression in the compiler where it no longer recognizes certain patterns as being able to be implemented using horizontal add/sub instructions. I have put the details in PR39195. Can you take a look?

Douglas Yung

> -----Original Message-----
> From: llvm-commits [mailto:llvm-commits-bounces at lists.llvm.org] On
> Behalf Of Sanjay Patel via llvm-commits
> Sent: Wednesday, October 03, 2018 14:45
> To: llvm-commits at lists.llvm.org
> Subject: [llvm] r343727 - [InstCombine] allow
> SimplifyDemandedVectorElts to work with FP binops
> 
> Author: spatel
> Date: Wed Oct  3 14:44:59 2018
> New Revision: 343727
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=343727&view=rev
> Log:
> [InstCombine] allow SimplifyDemandedVectorElts to work with FP binops
> 
> We're a long way from D50992 and D51553, but this is where we have to
> start.
> We weren't back-propagating undefs into binop constant values for
> anything but
> add/sub/mul/and/or/xor.
> 
> This is likely because we have to be careful about not introducing
> UB/poison
> with div/rem/shift. But I suspect we already are getting the poison
> part wrong
> for add/sub/mul (although it may not be possible to expose the bug
> currently
> because we use SimplifyDemandedVectorElts from a limited set of
> opcodes).
> See the discussion/implementation from D48987 and D49047.
> 
> This patch just enables functionality for FP ops because those do not
> have
> UB/poison potential.
> 
> Modified:
> 
> llvm/trunk/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
>     llvm/trunk/test/Transforms/InstCombine/X86/x86-avx512.ll
>     llvm/trunk/test/Transforms/InstCombine/shuffle_select.ll
>     llvm/trunk/test/Transforms/InstCombine/vec_demanded_elts.ll
>     llvm/trunk/test/Transforms/InstCombine/vec_shuffle.ll
> 
> Modified:
> llvm/trunk/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/lib/Transforms/InstCombine/InstCombineSimplifyDemand
> ed.cpp?rev=343727&r1=343726&r2=343727&view=diff
> =======================================================================
> =======
> ---
> llvm/trunk/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
> (original)
> +++
> llvm/trunk/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
> Wed Oct  3 14:44:59 2018
> @@ -1378,24 +1378,6 @@ Value *InstCombiner::SimplifyDemandedVec
>      }
>      break;
>    }
> -  case Instruction::And:
> -  case Instruction::Or:
> -  case Instruction::Xor:
> -  case Instruction::Add:
> -  case Instruction::Sub:
> -  case Instruction::Mul:
> -    // div/rem demand all inputs, because they don't want divide by
> zero.
> -    TmpV = SimplifyDemandedVectorElts(I->getOperand(0), DemandedElts,
> UndefElts,
> -                                      Depth + 1);
> -    if (TmpV) { I->setOperand(0, TmpV); MadeChange = true; }
> -    TmpV = SimplifyDemandedVectorElts(I->getOperand(1), DemandedElts,
> -                                      UndefElts2, Depth + 1);
> -    if (TmpV) { I->setOperand(1, TmpV); MadeChange = true; }
> -
> -    // Output elements are undefined if both are undefined.  Consider
> things
> -    // like undef&0.  The result is known zero, not undef.
> -    UndefElts &= UndefElts2;
> -    break;
>    case Instruction::FPTrunc:
>    case Instruction::FPExt:
>      TmpV = SimplifyDemandedVectorElts(I->getOperand(0), DemandedElts,
> UndefElts,
> @@ -1647,5 +1629,25 @@ Value *InstCombiner::SimplifyDemandedVec
>      break;
>    }
>    }
> +
> +  // TODO: We bail completely on integer div/rem and shifts because
> they have
> +  // UB/poison potential, but that should be refined.
> +  BinaryOperator *BO;
> +  if (match(I, m_BinOp(BO)) && !BO->isIntDivRem() && !BO->isShift()) {
> +    TmpV = SimplifyDemandedVectorElts(I->getOperand(0), DemandedElts,
> UndefElts,
> +                                      Depth + 1);
> +    if (TmpV) { I->setOperand(0, TmpV); MadeChange = true; }
> +    TmpV = SimplifyDemandedVectorElts(I->getOperand(1), DemandedElts,
> +                                      UndefElts2, Depth + 1);
> +    if (TmpV) { I->setOperand(1, TmpV); MadeChange = true; }
> +
> +    // TODO: If this is a potentially poison-producing instruction, we
> need
> +    // to drop the wrapping/exact flags?
> +
> +    // Output elements are undefined if both are undefined.  Consider
> things
> +    // like undef&0.  The result is known zero, not undef.
> +    UndefElts &= UndefElts2;
> +  }
> +
>    return MadeChange ? I : nullptr;
>  }
> 
> Modified: llvm/trunk/test/Transforms/InstCombine/X86/x86-avx512.ll
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/test/Transforms/InstCombine/X86/x86-
> avx512.ll?rev=343727&r1=343726&r2=343727&view=diff
> =======================================================================
> =======
> --- llvm/trunk/test/Transforms/InstCombine/X86/x86-avx512.ll (original)
> +++ llvm/trunk/test/Transforms/InstCombine/X86/x86-avx512.ll Wed Oct  3
> 14:44:59 2018
> @@ -1709,7 +1709,7 @@ define double @test_mask3_vfmsub_sd_1(<2
> 
>  define <4 x float> @test_mask3_vfnmsub_ss(<4 x float> %a, <4 x float>
> %b, <4 x float> %c, i8 %mask) {
>  ; CHECK-LABEL: @test_mask3_vfnmsub_ss(
> -; CHECK-NEXT:    [[DOTRHS:%.*]] = extractelement <4 x float>
> [[A:%.*]], i32 0
> +; CHECK-NEXT:    [[DOTRHS:%.*]] = extractelement <4 x float>
> [[A:%.*]], i64 0
>  ; CHECK-NEXT:    [[TMP1:%.*]] = fsub float -0.000000e+00, [[DOTRHS]]
>  ; CHECK-NEXT:    [[TMP2:%.*]] = extractelement <4 x float> [[B:%.*]],
> i32 0
>  ; CHECK-NEXT:    [[DOTRHS1:%.*]] = extractelement <4 x float>
> [[C:%.*]], i64 0
> 
> Modified: llvm/trunk/test/Transforms/InstCombine/shuffle_select.ll
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/test/Transforms/InstCombine/shuffle_select.ll?rev=34
> 3727&r1=343726&r2=343727&view=diff
> =======================================================================
> =======
> --- llvm/trunk/test/Transforms/InstCombine/shuffle_select.ll (original)
> +++ llvm/trunk/test/Transforms/InstCombine/shuffle_select.ll Wed Oct  3
> 14:44:59 2018
> @@ -348,7 +348,7 @@ define <4 x float> @fadd(<4 x float> %v)
> 
>  define <4 x double> @fsub(<4 x double> %v) {
>  ; CHECK-LABEL: @fsub(
> -; CHECK-NEXT:    [[B:%.*]] = fsub <4 x double> <double 4.100000e+01,
> double 4.200000e+01, double 4.300000e+01, double 4.400000e+01>,
> [[V:%.*]]
> +; CHECK-NEXT:    [[B:%.*]] = fsub <4 x double> <double undef, double
> undef, double 4.300000e+01, double 4.400000e+01>, [[V:%.*]]
>  ; CHECK-NEXT:    [[S:%.*]] = shufflevector <4 x double> [[V]], <4 x
> double> [[B]], <4 x i32> <i32 undef, i32 1, i32 6, i32 7>
>  ; CHECK-NEXT:    ret <4 x double> [[S]]
>  ;
> @@ -371,7 +371,7 @@ define <4 x float> @fmul(<4 x float> %v)
> 
>  define <4 x double> @fdiv_constant_op0(<4 x double> %v) {
>  ; CHECK-LABEL: @fdiv_constant_op0(
> -; CHECK-NEXT:    [[B:%.*]] = fdiv fast <4 x double> <double
> 4.100000e+01, double 4.200000e+01, double 4.300000e+01, double
> 4.400000e+01>, [[V:%.*]]
> +; CHECK-NEXT:    [[B:%.*]] = fdiv fast <4 x double> <double undef,
> double undef, double 4.300000e+01, double 4.400000e+01>, [[V:%.*]]
>  ; CHECK-NEXT:    [[S:%.*]] = shufflevector <4 x double> [[V]], <4 x
> double> [[B]], <4 x i32> <i32 undef, i32 1, i32 6, i32 7>
>  ; CHECK-NEXT:    ret <4 x double> [[S]]
>  ;
> @@ -392,7 +392,7 @@ define <4 x double> @fdiv_constant_op1(<
> 
>  define <4 x double> @frem(<4 x double> %v) {
>  ; CHECK-LABEL: @frem(
> -; CHECK-NEXT:    [[B:%.*]] = frem <4 x double> <double 4.100000e+01,
> double 4.200000e+01, double 4.300000e+01, double 4.400000e+01>,
> [[V:%.*]]
> +; CHECK-NEXT:    [[B:%.*]] = frem <4 x double> <double 4.100000e+01,
> double 4.200000e+01, double undef, double undef>, [[V:%.*]]
>  ; CHECK-NEXT:    [[S:%.*]] = shufflevector <4 x double> [[B]], <4 x
> double> [[V]], <4 x i32> <i32 0, i32 1, i32 6, i32 7>
>  ; CHECK-NEXT:    ret <4 x double> [[S]]
>  ;
> @@ -791,8 +791,8 @@ define <4 x double> @fdiv_fdiv(<4 x doub
> 
>  define <4 x double> @frem_frem(<4 x double> %v0) {
>  ; CHECK-LABEL: @frem_frem(
> -; CHECK-NEXT:    [[T1:%.*]] = frem <4 x double> <double 1.000000e+00,
> double 2.000000e+00, double 3.000000e+00, double 4.000000e+00>,
> [[V0:%.*]]
> -; CHECK-NEXT:    [[T2:%.*]] = frem <4 x double> [[V0]], <double
> 5.000000e+00, double 6.000000e+00, double 7.000000e+00, double
> 8.000000e+00>
> +; CHECK-NEXT:    [[T1:%.*]] = frem <4 x double> <double 1.000000e+00,
> double 2.000000e+00, double undef, double undef>, [[V0:%.*]]
> +; CHECK-NEXT:    [[T2:%.*]] = frem <4 x double> [[V0]], <double undef,
> double undef, double 7.000000e+00, double 8.000000e+00>
>  ; CHECK-NEXT:    [[T3:%.*]] = shufflevector <4 x double> [[T1]], <4 x
> double> [[T2]], <4 x i32> <i32 0, i32 1, i32 6, i32 7>
>  ; CHECK-NEXT:    ret <4 x double> [[T3]]
>  ;
> @@ -1284,8 +1284,8 @@ define <4 x double> @frem_2_vars(<4 x do
> 
>  define <4 x double> @fdiv_2_vars(<4 x double> %v0, <4 x double> %v1) {
>  ; CHECK-LABEL: @fdiv_2_vars(
> -; CHECK-NEXT:    [[T1:%.*]] = fdiv <4 x double> <double 1.000000e+00,
> double 2.000000e+00, double 3.000000e+00, double 4.000000e+00>,
> [[V0:%.*]]
> -; CHECK-NEXT:    [[T2:%.*]] = fdiv <4 x double> [[V1:%.*]], <double
> 5.000000e+00, double 6.000000e+00, double 7.000000e+00, double
> 8.000000e+00>
> +; CHECK-NEXT:    [[T1:%.*]] = fdiv <4 x double> <double 1.000000e+00,
> double 2.000000e+00, double undef, double undef>, [[V0:%.*]]
> +; CHECK-NEXT:    [[T2:%.*]] = fdiv <4 x double> [[V1:%.*]], <double
> undef, double undef, double 7.000000e+00, double 8.000000e+00>
>  ; CHECK-NEXT:    [[T3:%.*]] = shufflevector <4 x double> [[T1]], <4 x
> double> [[T2]], <4 x i32> <i32 0, i32 1, i32 6, i32 7>
>  ; CHECK-NEXT:    ret <4 x double> [[T3]]
>  ;
> 
> Modified: llvm/trunk/test/Transforms/InstCombine/vec_demanded_elts.ll
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/test/Transforms/InstCombine/vec_demanded_elts.ll?rev
> =343727&r1=343726&r2=343727&view=diff
> =======================================================================
> =======
> --- llvm/trunk/test/Transforms/InstCombine/vec_demanded_elts.ll
> (original)
> +++ llvm/trunk/test/Transforms/InstCombine/vec_demanded_elts.ll Wed Oct
> 3 14:44:59 2018
> @@ -421,7 +421,7 @@ define <3 x i8> @shuf_urem_const_op1(<3
> 
>  define <3 x float> @shuf_fadd(<3 x float> %x) {
>  ; CHECK-LABEL: @shuf_fadd(
> -; CHECK-NEXT:    [[BO:%.*]] = fadd <3 x float> [[X:%.*]], <float
> 1.000000e+00, float 2.000000e+00, float 3.000000e+00>
> +; CHECK-NEXT:    [[BO:%.*]] = fadd <3 x float> [[X:%.*]], <float
> 1.000000e+00, float 2.000000e+00, float undef>
>  ; CHECK-NEXT:    [[R:%.*]] = shufflevector <3 x float> [[BO]], <3 x
> float> undef, <3 x i32> <i32 undef, i32 1, i32 0>
>  ; CHECK-NEXT:    ret <3 x float> [[R]]
>  ;
> @@ -432,7 +432,7 @@ define <3 x float> @shuf_fadd(<3 x float
> 
>  define <3 x float> @shuf_fsub(<3 x float> %x) {
>  ; CHECK-LABEL: @shuf_fsub(
> -; CHECK-NEXT:    [[BO:%.*]] = fsub fast <3 x float> <float
> 1.000000e+00, float 2.000000e+00, float 3.000000e+00>, [[X:%.*]]
> +; CHECK-NEXT:    [[BO:%.*]] = fsub fast <3 x float> <float
> 1.000000e+00, float undef, float 3.000000e+00>, [[X:%.*]]
>  ; CHECK-NEXT:    [[R:%.*]] = shufflevector <3 x float> [[BO]], <3 x
> float> undef, <3 x i32> <i32 undef, i32 0, i32 2>
>  ; CHECK-NEXT:    ret <3 x float> [[R]]
>  ;
> @@ -443,7 +443,7 @@ define <3 x float> @shuf_fsub(<3 x float
> 
>  define <3 x float> @shuf_fmul(<3 x float> %x) {
>  ; CHECK-LABEL: @shuf_fmul(
> -; CHECK-NEXT:    [[BO:%.*]] = fmul reassoc <3 x float> [[X:%.*]],
> <float 1.000000e+00, float 2.000000e+00, float 3.000000e+00>
> +; CHECK-NEXT:    [[BO:%.*]] = fmul reassoc <3 x float> [[X:%.*]],
> <float 1.000000e+00, float 2.000000e+00, float undef>
>  ; CHECK-NEXT:    [[R:%.*]] = shufflevector <3 x float> [[BO]], <3 x
> float> undef, <3 x i32> <i32 undef, i32 1, i32 0>
>  ; CHECK-NEXT:    ret <3 x float> [[R]]
>  ;
> @@ -454,7 +454,7 @@ define <3 x float> @shuf_fmul(<3 x float
> 
>  define <3 x float> @shuf_fdiv_const_op0(<3 x float> %x) {
>  ; CHECK-LABEL: @shuf_fdiv_const_op0(
> -; CHECK-NEXT:    [[BO:%.*]] = fdiv reassoc ninf <3 x float> <float
> 1.000000e+00, float 2.000000e+00, float 3.000000e+00>, [[X:%.*]]
> +; CHECK-NEXT:    [[BO:%.*]] = fdiv reassoc ninf <3 x float> <float
> 1.000000e+00, float undef, float 3.000000e+00>, [[X:%.*]]
>  ; CHECK-NEXT:    [[R:%.*]] = shufflevector <3 x float> [[BO]], <3 x
> float> undef, <3 x i32> <i32 undef, i32 0, i32 2>
>  ; CHECK-NEXT:    ret <3 x float> [[R]]
>  ;
> @@ -465,7 +465,7 @@ define <3 x float> @shuf_fdiv_const_op0(
> 
>  define <3 x float> @shuf_fdiv_const_op1(<3 x float> %x) {
>  ; CHECK-LABEL: @shuf_fdiv_const_op1(
> -; CHECK-NEXT:    [[BO:%.*]] = fdiv nnan ninf <3 x float> [[X:%.*]],
> <float 1.000000e+00, float 2.000000e+00, float 3.000000e+00>
> +; CHECK-NEXT:    [[BO:%.*]] = fdiv nnan ninf <3 x float> [[X:%.*]],
> <float 1.000000e+00, float 2.000000e+00, float undef>
>  ; CHECK-NEXT:    [[R:%.*]] = shufflevector <3 x float> [[BO]], <3 x
> float> undef, <3 x i32> <i32 undef, i32 1, i32 0>
>  ; CHECK-NEXT:    ret <3 x float> [[R]]
>  ;
> @@ -476,7 +476,7 @@ define <3 x float> @shuf_fdiv_const_op1(
> 
>  define <3 x float> @shuf_frem_const_op0(<3 x float> %x) {
>  ; CHECK-LABEL: @shuf_frem_const_op0(
> -; CHECK-NEXT:    [[BO:%.*]] = frem nnan <3 x float> <float
> 1.000000e+00, float 2.000000e+00, float 3.000000e+00>, [[X:%.*]]
> +; CHECK-NEXT:    [[BO:%.*]] = frem nnan <3 x float> <float
> 1.000000e+00, float undef, float 3.000000e+00>, [[X:%.*]]
>  ; CHECK-NEXT:    [[R:%.*]] = shufflevector <3 x float> [[BO]], <3 x
> float> undef, <3 x i32> <i32 undef, i32 2, i32 0>
>  ; CHECK-NEXT:    ret <3 x float> [[R]]
>  ;
> @@ -487,7 +487,7 @@ define <3 x float> @shuf_frem_const_op0(
> 
>  define <3 x float> @shuf_frem_const_op1(<3 x float> %x) {
>  ; CHECK-LABEL: @shuf_frem_const_op1(
> -; CHECK-NEXT:    [[BO:%.*]] = frem reassoc ninf <3 x float> [[X:%.*]],
> <float 1.000000e+00, float 2.000000e+00, float 3.000000e+00>
> +; CHECK-NEXT:    [[BO:%.*]] = frem reassoc ninf <3 x float> [[X:%.*]],
> <float undef, float 2.000000e+00, float 3.000000e+00>
>  ; CHECK-NEXT:    [[R:%.*]] = shufflevector <3 x float> [[BO]], <3 x
> float> undef, <3 x i32> <i32 1, i32 undef, i32 2>
>  ; CHECK-NEXT:    ret <3 x float> [[R]]
>  ;
> 
> Modified: llvm/trunk/test/Transforms/InstCombine/vec_shuffle.ll
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/test/Transforms/InstCombine/vec_shuffle.ll?rev=34372
> 7&r1=343726&r2=343727&view=diff
> =======================================================================
> =======
> --- llvm/trunk/test/Transforms/InstCombine/vec_shuffle.ll (original)
> +++ llvm/trunk/test/Transforms/InstCombine/vec_shuffle.ll Wed Oct  3
> 14:44:59 2018
> @@ -952,7 +952,7 @@ define <2 x float> @fsub_splat_constant0
> 
>  define <2 x float> @fsub_splat_constant1(<2 x float> %x) {
>  ; CHECK-LABEL: @fsub_splat_constant1(
> -; CHECK-NEXT:    [[TMP1:%.*]] = fadd <2 x float> [[X:%.*]], <float -
> 4.200000e+01, float 0x7FF8000000000000>
> +; CHECK-NEXT:    [[TMP1:%.*]] = fadd <2 x float> [[X:%.*]], <float -
> 4.200000e+01, float undef>
>  ; CHECK-NEXT:    [[R:%.*]] = shufflevector <2 x float> [[TMP1]], <2 x
> float> undef, <2 x i32> zeroinitializer
>  ; CHECK-NEXT:    ret <2 x float> [[R]]
>  ;
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list