[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;
> 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