[llvm-commits] [llvm] r149226 - /llvm/trunk/lib/VMCore/Constants.cpp

[Chris Lattner]

On Feb 9, 2012, at 6:06 AM, Duncan Sands wrote:
>> +      if (CI->getType()->isIntegerTy(8)) {
>> +        SmallVector<uint8_t, 16>  Elts;
>> +        for (unsigned i = 0, e = V.size(); i != e; ++i)
>> +          if (ConstantInt *CI = dyn_cast<ConstantInt>(V[i]))
>> +            Elts.push_back(CI->getZExtValue());
>> +          else
>> +            break;
> 
> here you know the first element of V is a ConstantInt, since that element is C
> and you checked C above.  So this could be written like this instead
> 
>         SmallVector<uint8_t, 16>  Elts;
>         Elts.push_back(CI->getZExtValue());
>         for (unsigned i = 1, e = V.size(); i < e; ++i)
>           if (ConstantInt *CI = dyn_cast<ConstantInt>(V[i]))
>             Elts.push_back(CI->getZExtValue());
>           else
>             break;
> 
> saving one dyn_cast.  Maybe not worth it.

True, but I don't think it is worth it.

> ...
>> +    }
>> 
>> +    if (ConstantFP *CFP = dyn_cast<ConstantFP>(C)) {
> 
> This could be
>     } else if (ConstantFP *CFP = dyn_cast<ConstantFP>(C)) {
> saving a pointless dyn_cast if you got here from the ConstantInt case.

These dyn_cast's compile down into "if (C->getKind() == 1234)", so I expect jump threading to be able to handle it.  The only case that probably won't get threaded is if none of the int cases hit, which isn't common.  I'd rather keep the code simple,

-Chris