[llvm-commits] A patch fixing # 9173
Duncan Sands
baldrick at free.fr
Wed Feb 9 08:18:18 PST 2011
Hi Nadav,
@@ -132,11 +132,27 @@
if (const VectorType *SrcTy = dyn_cast<VectorType>(V->getType())) {
assert(DestPTy->getBitWidth() == SrcTy->getBitWidth() &&
"Not cast between same sized vectors!");
- SrcTy = NULL;
// First, check for null. Undef is already handled.
if (isa<ConstantAggregateZero>(V))
return Constant::getNullValue(DestTy);
+ // In case of all-one vector, create a new all-one of dst type
If you are only going to do this when the source is an all-ones vector then
logic belongs in BitCastConstantVector. You could also handle the case when
the source is an all-ones integer in which case at least that part belongs
here.
+ if (ConstantVector* CV = dyn_cast<ConstantVector>(V)) {
+ bool AllOne = true;
+ for (unsigned int i=0; i< SrcTy->getNumElements() ; ++i) {
+ ConstantInt *ci = dyn_cast<ConstantInt>(CV->getOperand(i));
+ if (! ci) {
+ AllOne = false;
+ break;
+ }
+ if (! ci->isAllOnesValue()) {
+ AllOne = false;
+ break;
+ }
+ }
This test for all-ones can be replaced with: CV->isAllOnesValue()
+ if (AllOne) Constant::getAllOnesValue(DestPTy);
Looks like you are missing the "return" keyword.
@@ -694,15 +710,37 @@
if (isa<UndefValue>(Cond)) return V1;
if (V1 == V2) return V1;
+ if (Cond->isNullValue()) return V2;
You could also check if CondV->isAllOnesValue() here and if so return V1.
You could also replace the ConstantInt Cond check at the start of this method
with something that checks isNullValue and isAllOnesValue.
+
+ if (ConstantVector* CondV = dyn_cast<ConstantVector>(Cond)) {
+ const VectorType *VTy = CondV->getType();
+ ConstantVector *CP1 = dyn_cast<ConstantVector>(V1);
+ ConstantVector *CP2 = dyn_cast<ConstantVector>(V2);
+
+ if ((CP1 != NULL || isa<ConstantAggregateZero>(V1)) &&
+ (CP2 != NULL || isa<ConstantAggregateZero>(V2))) {
Is it possible for this check to fail? If not, it should be an assertion.
+
+ const Type *EltTy = V1->getType();
+ std::vector<Constant*> Res;
You know the size that Res will be so you can reserve the right amount of space
here.
+ for (unsigned i = 0, e = VTy->getNumElements(); i != e; ++i) {
+ ConstantInt* c = dyn_cast<ConstantInt>(CondV->getOperand(i));
What if CondV->getOperand(i) is a ConstantExpr and not a ConstantInt? Then
you will get null here and crash below doing c->getZExtValue().
+ Constant *C1 = CP1 ? CP1->getOperand(i) : Constant::getNullValue(EltTy);
+ Constant *C2 = CP2 ? CP2->getOperand(i) : Constant::getNullValue(EltTy);
+ Res.push_back(c->getZExtValue() ? C1 : C2);
+ }
+ return ConstantVector::get(Res);
+ }
+ }
+
if (ConstantExpr *TrueVal = dyn_cast<ConstantExpr>(V1)) {
if (TrueVal->getOpcode() == Instruction::Select)
if (TrueVal->getOperand(0) == Cond)
- return ConstantExpr::getSelect(Cond, TrueVal->getOperand(1), V2);
+ return ConstantExpr::getSelect(Cond, TrueVal->getOperand(1), V2);
What's the reason for changing this line?
}
if (ConstantExpr *FalseVal = dyn_cast<ConstantExpr>(V2)) {
if (FalseVal->getOpcode() == Instruction::Select)
if (FalseVal->getOperand(0) == Cond)
- return ConstantExpr::getSelect(Cond, V1, FalseVal->getOperand(2));
+ return ConstantExpr::getSelect(Cond, V1, FalseVal->getOperand(2));
Likewise.
Ciao, Duncan.
More information about the llvm-commits
mailing list