[llvm-commits] [llvm] r85814 - /llvm/trunk/lib/Analysis/MemoryBuiltins.cpp

[Chris Lattner]

On Nov 2, 2009, at 10:51 AM, Victor Hernandez wrote:
> URL: http://llvm.org/viewvc/llvm-project?rev=85814&view=rev
> Log:
> Set bit instead of calling pow() to compute 2 << n

Thanks Victor,

> +++ llvm/trunk/lib/Analysis/MemoryBuiltins.cpp Mon Nov  2 12:51:28  
> 2009
> @@ -16,6 +16,7 @@
> #include "llvm/Constants.h"
> #include "llvm/Instructions.h"
> #include "llvm/Module.h"
> +#include "llvm/ADT/APInt.h"

You don't need this #include, please remove it.

> @@ -156,15 +157,22 @@
>         return Op1;
>     }
>     if (Opcode == Instruction::Shl) {
> -      ConstantInt* Op1Int = dyn_cast<ConstantInt>(Op1);
> -      if (!Op1Int) return NULL;
> -      Value* Op1Pow = ConstantInt::get(Op1->getType(), (uint64_t)
> -                                    pow(2.0, (double) Op1Int- 
> >getZExtValue()));
> +      ConstantInt* Op1CI = dyn_cast<ConstantInt>(Op1);
> +      if (!Op1CI) return NULL;
> +
> +      APInt Op1Int = Op1CI->getValue();
> +      unsigned Op1Width = Op1Int.getBitWidth();
> +      // check for overflow
> +      if (Op1Int.getActiveBits() > 64 || Op1Int.getZExtValue() >  
> Op1Width)
> +        return NULL;

If the shift overflows, the original code is undefined.  Please just  
use the APInt::getLimitedValue method  (with the bitwidth as the  
argument) to handle this.  You'll end up with much more elegant code.

Finally, please use:

   ConstantInt *Op1CI
instead of:
   ConstantInt* Op1CI

With Op1CI and many other variables in this file.

Thanks,

-Chris



> +      Value* Op1Pow = ConstantInt::get(Context,
> +                                 APInt(Op1Width, 0).set 
> (Op1Int.getZExtValue()));
> +
>       if (Op0 == ElementSize || (FoldedElementSize && Op0 ==  
> FoldedElementSize))
>         // ArraySize << log2(ElementSize)
>         return Op1Pow;
>       if (Op1Pow == ElementSize ||
> -        (FoldedElementSize && Op1Pow == FoldedElementSize))
> +          (FoldedElementSize && Op1Pow == FoldedElementSize))
>         // ElementSize << log2(ArraySize)
>         return Op0;
>     }
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits