[PATCH] D31189: [InstCombine] Fix folding of select into phi nodes when incoming element of phi is a vector type

[Anna Thomas via Phabricator via llvm-commits]

anna added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:911-912
       Constant *InC = dyn_cast<Constant>(PN->getIncomingValue(i));
-      if (InC && !isa<ConstantExpr>(InC))
+      // For vector constants, we cannot use isNullValue to fold into
+      // FalseVInPred versus TrueVInPred. It does not investigate the `nullness`
+      // of individual elements in the vector (and will always return false).
+      if (InC && !isa<ConstantExpr>(InC) && !isa<VectorType>(InC->getType()))
         InV = InC->isNullValue() ? FalseVInPred : TrueVInPred;
       else
----------------
majnemer wrote:
> I am surprised, what is InC if not a ConstantAggregateZero in this case?
`InC` is a ConstantAggregateZero is all vector elements are zero. In that case, `isNullValue` will return true and it is correct to fold into `FalseVInPred`.

However, if we have InC as a vector <0,1,1,0>, `isNullValue` will return false which is also correct, but we will fold into `TrueVInPred` which is an incorrect fold. 

I think I'll have to update my comment which states `always returns false`. The problem is the incorrect fold when we have non-zero elements in the vector. 

For example, in vec1 test case, without this change, we will incorrectly return `ret <4 x i64> zeroinitializer`.


https://reviews.llvm.org/D31189