[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 llvm-commits at lists.llvm.org
Tue Mar 21 08:59:30 PDT 2017

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;
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`.


More information about the llvm-commits mailing list