[llvm-commits] [llvm] r164684 - in /llvm/trunk: lib/Transforms/Utils/SimplifyCFG.cpp test/Transforms/SimplifyCFG/switch_to_lookup_table.ll

[Hans Wennborg]

Thanks for the review! I've attached a patch which addresses your
comments. Please take a look.

 - Hans

On Wed, Sep 26, 2012 at 1:17 PM, Duncan Sands <baldrick at free.fr> wrote:
>> SimplifyCFG: Make the switch-to-lookup table transformation store the
>> tables in bitmaps when they fit in a target-legal register.
>
> very nice.

Thanks!

>> +STATISTIC(NumBitMaps, "Number of switch instructions turned into
>> bitmaps");
>>   STATISTIC(NumSinkCommons, "Number of common instructions sunk down to
>> the end block");
>
> Not in alphabetical order (very nitpicky I know!).

Fixed.

>> +    for (uint64_t I = TableSize; I > 0; --I) {
>> +      TableInt <<= IT->getBitWidth();
>> +      TableInt |= cast<ConstantInt>(TableContents[I -
>> 1])->getZExtValue();
>
>
> This getZextValue may crash on a (future) target with 128 bit registers.
> Why
> not just use apints all the way:
>
>   TableInt |= cast<ConstantInt>(TableContents[I -
> 1])->getValue().zext(TableInt.getBitWidth());

Fixed.

>> +      // Cast Index to the same type as the bitmap.
>> +      // Note: The Index is <= the number of elements in the table, so
>> +      // truncating it to the width of the bitmask is safe.
>> +      Value *ShiftAmt = Index;
>> +      IntegerType *IndexTy = cast<IntegerType>(Index->getType());
>> +      if (IndexTy->getBitWidth() < MapTy->getBitWidth())
>> +        ShiftAmt = Builder.CreateZExt(ShiftAmt, MapTy, "switch.zext");
>> +      else if (IndexTy->getBitWidth() > MapTy->getBitWidth())
>> +        ShiftAmt = Builder.CreateTrunc(ShiftAmt, MapTy, "switch.trunc");
>
>
> How about adding a CreateZExtOrTrunc method to IRBuilder and using that
> instead (in which case please add CreateSExtOrTrunc too)?

That sounds like a good idea!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fixes.patch
Type: application/octet-stream
Size: 4918 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120926/c4a1a003/attachment.obj>