[InstCombine] Allow undef vectors when foldSelectToCopysign

[Chenbing.Zheng]

Summary

In function foldSelectToCopysign, we can allow undefs when
matching vectors.

[spatel]

This does not look correct.

In the example test, if both elements of X are negative, we should return <-42.0, -42.0>. But after this transform, we will return <-42.0, undef>?

[spatel]

In D125671#3516360, @spatel wrote:

This does not look correct.

In the example test, if both elements of X are negative, we should return <-42.0, -42.0>. But after this transform, we will return <-42.0, undef>?

Sorry - I missed the invert / fneg. That should be:

In the example test, if both elements of X are positive, we should return <-42.0, -42.0>. But after this transform, we will return <-42.0, undef>?

[Chenbing.Zheng]

address comment

[spatel]

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

2306–2307

This might be more efficient and easier to read:

Value *MagArg = ConstantFP::get(SelType, abs(*TC));

llvm/test/Transforms/InstCombine/select.ll

1710

Test coverage would be better if you vary the predicate (for example ule). Also, swap the positive and negative constants on the next test?

[Chenbing.Zheng]

address comments

[spatel]

LGTM - it might also be possible to allow undefs in the compare constant, but we would have to test that carefully with Alive2 to be sure.

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

static Instruction *foldSelectToCopysign(SelectInst &Sel,
   Value *Cond = Sel.getCondition();
   Value *TVal = Sel.getTrueValue();
   Value *FVal = Sel.getFalseValue();
   Type *SelType = Sel.getType();
 
   // Match select ?, TC, FC where the constants are equal but negated.
   // TODO: Generalize to handle a negated variable operand?
   const APFloat *TC, *FC;
-  if (!match(TVal, m_APFloat(TC)) || !match(FVal, m_APFloat(FC)) ||
+  if (!match(TVal, m_APFloatAllowUndef(TC)) ||
+      !match(FVal, m_APFloatAllowUndef(FC)) ||
       !abs(*TC).bitwiseIsEqual(abs(*FC)))
     return nullptr;
 
   assert(TC != FC && "Expected equal select arms to simplify");
 
   Value *X;
   const APInt *C;
   bool IsTrueIfSignSet;
static Instruction *foldSelectToCopysign(SelectInst &Sel,
   // (bitcast X) >= 0 ? -TC :  TC --> copysign(TC, -X)
   // (bitcast X) >= 0 ?  TC : -TC --> copysign(TC,  X)
   // Note: FMF from the select can not be propagated to the new instructions.
   if (IsTrueIfSignSet ^ TC->isNegative())
     X = Builder.CreateFNeg(X);
 
   // Canonicalize the magnitude argument as the positive constant since we do
   // not care about its sign.
-  Value *MagArg = TC->isNegative() ? FVal : TVal;
+  Value *MagArg = ConstantFP::get(SelType, abs(*TC));
   Function *F = Intrinsic::getDeclaration(Sel.getModule(), Intrinsic::copysign,
                                           Sel.getType());
   return CallInst::Create(F, { MagArg, X });
 }
 
 Instruction *InstCombinerImpl::foldVectorSelect(SelectInst &Sel) {
   auto *VecTy = dyn_cast<FixedVectorType>(Sel.getType());
   if (!VecTy)
     return nullptr;

llvm/test/Transforms/InstCombine/select.ll

 ; CHECK-NEXT:    ret float [[R]]
 ;
   %i = bitcast float %x to i32
   %ispos = icmp ult i32 %i, 2147483648
   %r = select fast i1 %ispos, float -43.0, float 43.0
   ret float %r
 }
 
-; TODO: Allow undefs when matching vectors.
-
 define <2 x float> @copysign_vec_undef(<2 x float> %x) {
 ; CHECK-LABEL: @copysign_vec_undef(
-; CHECK-NEXT:    [[I:%.*]] = bitcast <2 x float> [[X:%.*]] to <2 x i32>
-; CHECK-NEXT:    [[ISNEG:%.*]] = icmp slt <2 x i32> [[I]], zeroinitializer
-; CHECK-NEXT:    [[R:%.*]] = select nnan arcp <2 x i1> [[ISNEG]], <2 x float> <float 4.200000e+01, float undef>, <2 x float> <float -4.200000e+01, float -4.200000e+01>
+; CHECK-NEXT:    [[TMP1:%.*]] = fneg <2 x float> [[X:%.*]]
+; CHECK-NEXT:    [[R:%.*]] = call <2 x float> @llvm.copysign.v2f32(<2 x float> <float 4.200000e+01, float 4.200000e+01>, <2 x float> [[TMP1]])
 ; CHECK-NEXT:    ret <2 x float> [[R]]
 ;
   %i = bitcast <2 x float> %x to <2 x i32>
   %isneg = icmp ugt <2 x i32> %i, <i32 2147483647, i32 2147483647>
   %r = select arcp nnan <2 x i1> %isneg, <2 x float> <float 42.0, float undef>, <2 x float> <float -42.0, float -42.0>
   ret <2 x float> %r
 }
 
 define <2 x float> @copysign_vec_undef1(<2 x float> %x) {
 ; CHECK-LABEL: @copysign_vec_undef1(
-; CHECK-NEXT:    [[I:%.*]] = bitcast <2 x float> [[X:%.*]] to <2 x i32>
-; CHECK-NEXT:    [[ISNEG:%.*]] = icmp slt <2 x i32> [[I]], zeroinitializer
-; CHECK-NEXT:    [[R:%.*]] = select nnan arcp <2 x i1> [[ISNEG]], <2 x float> <float 4.200000e+01, float 4.200000e+01>, <2 x float> <float undef, float -4.200000e+01>
+; CHECK-NEXT:    [[R:%.*]] = call <2 x float> @llvm.copysign.v2f32(<2 x float> <float 4.200000e+01, float 4.200000e+01>, <2 x float> [[X:%.*]])
 ; CHECK-NEXT:    ret <2 x float> [[R]]
 ;
   %i = bitcast <2 x float> %x to <2 x i32>
-  %isneg = icmp ugt <2 x i32> %i, <i32 2147483647, i32 2147483647>
+  %isneg = icmp ult <2 x i32> %i, <i32 2147483648, i32 2147483648>
   %r = select arcp nnan <2 x i1> %isneg, <2 x float> <float 42.0, float 42.0>, <2 x float> <float undef, float -42.0>
   ret <2 x float> %r
 }
 
 define <2 x float> @copysign_vec_undef3(<2 x float> %x) {
 ; CHECK-LABEL: @copysign_vec_undef3(
-; CHECK-NEXT:    [[I:%.*]] = bitcast <2 x float> [[X:%.*]] to <2 x i32>
-; CHECK-NEXT:    [[ISNEG:%.*]] = icmp slt <2 x i32> [[I]], zeroinitializer
-; CHECK-NEXT:    [[R:%.*]] = select nnan arcp <2 x i1> [[ISNEG]], <2 x float> <float 4.200000e+01, float undef>, <2 x float> <float -4.200000e+01, float undef>
+; CHECK-NEXT:    [[R:%.*]] = call <2 x float> @llvm.copysign.v2f32(<2 x float> <float 4.200000e+01, float 4.200000e+01>, <2 x float> [[X:%.*]])
 ; CHECK-NEXT:    ret <2 x float> [[R]]
 ;
   %i = bitcast <2 x float> %x to <2 x i32>
   %isneg = icmp ugt <2 x i32> %i, <i32 2147483647, i32 2147483647>
-  %r = select arcp nnan <2 x i1> %isneg, <2 x float> <float 42.0, float undef>, <2 x float> <float -42.0, float undef>
+  %r = select arcp nnan <2 x i1> %isneg, <2 x float> <float -42.0, float undef>, <2 x float> <float +42.0, float undef>
   ret <2 x float> %r
 }
 
 declare void @use1(i1)
 
 ; Negative test
 
 define float @copysign_extra_use(float %x) {