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

[Chandler Carruth]

On Sun, Jul 11, 2010 at 11:31 PM, Chandler Carruth <chandlerc at google.com> wrote:
> On Sun, Jul 11, 2010 at 5:22 PM, Chris Lattner <sabre at nondot.org> wrote:
>> Author: lattner
>> Date: Sun Jul 11 19:22:51 2010
>> New Revision: 108113
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=108113&view=rev
>> Log:
>> fix PR7429, a crash turning a load from a string into a float.
>>
>> Modified:
>>    llvm/trunk/lib/Analysis/ConstantFolding.cpp
>>    llvm/trunk/test/Transforms/InstCombine/load3.ll
>>
>> Modified: llvm/trunk/lib/Analysis/ConstantFolding.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ConstantFolding.cpp?rev=108113&r1=108112&r2=108113&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Analysis/ConstantFolding.cpp (original)
>> +++ 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!