[llvm-commits] [llvm] r108113 - in /llvm/trunk: lib/Analysis/ConstantFolding.cpp test/Transforms/InstCombine/load3.ll

[Chris Lattner]

On Jul 11, 2010, at 11:49 PM, Chandler Carruth wrote:

>>> +++ llvm/trunk/lib/Analysis/ConstantFolding.cpp Sun Jul 11 19:22:51 2010
>>> @@ -436,8 +436,10 @@
>>>     unsigned StrLen = Str.length();
>>>     const Type *Ty = cast<PointerType>(CE->getType())->getElementType();
>>>     unsigned NumBits = Ty->getPrimitiveSizeInBits();
>>> -    // Replace LI with immediate integer store.
>>> -    if ((NumBits >> 3) == StrLen + 1) {
>>> +    // Replace load with immediate integer if the result is an integer or fp
>>> +    // value.
>>> +    if ((NumBits >> 3) == StrLen + 1 && (NumBits & 7) == 0 &&
>>> +        isa<IntegerType>(Ty) || Ty->isFloatingPointTy()) {
>> 
>> GCC warns about the && and || operators being used w/o parentheses.
>> Sometimes, this is just pedantic, but did you mean to have ()s around
>> the 'isa<IntegerType>(Ty) || Ty->isFloatingPointTy()' here? As it
>> stands, won't an fp value bypass the checks on NumBits?
> 
> After sanity checking with Nick on IRC, I submitted a patch adding
> these parens in r 108129. Reading the code made me much more confident
> that this was just an oversight. Let me know if I broke anything!

Yep, that was the right fix, thanks!