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

[Duncan Sands]

Hi Hans,

> SimplifyCFG: Make the switch-to-lookup table transformation store the
> tables in bitmaps when they fit in a target-legal register.

very nice.

> --- llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp Wed Sep 26 04:44:49 2012
> @@ -60,6 +60,7 @@
>
>   STATISTIC(NumSpeculations, "Number of speculative executed instructions");
>   STATISTIC(NumLookupTables, "Number of switch instructions turned into lookup tables");
> +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!).

>
>   namespace {
> @@ -3283,7 +3300,8 @@
>                                        uint64_t TableSize,
>                                        ConstantInt *Offset,
>                  const SmallVector<std::pair<ConstantInt*, Constant*>, 4>& Values,
> -                                     Constant *DefaultValue) {
> +                                     Constant *DefaultValue,
> +                                     const TargetData *TD) {
>     assert(Values.size() && "Can't build lookup table without values.");
>     assert(TableSize >= Values.size() && "Can't fit values in table.");
>
> @@ -3323,6 +3341,21 @@
>       return;
>     }
>
> +  // If the type is integer and the table fits in a register, build a bitmap.
> +  if (WouldFitInRegister(TD, TableSize, DefaultValue->getType())) {
> +    IntegerType *IT = cast<IntegerType>(DefaultValue->getType());
> +    APInt TableInt(TableSize * IT->getBitWidth(), 0);
> +    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());

?

> +    }
> +    BitMap = ConstantInt::get(M.getContext(), TableInt);
> +    BitMapElementTy = IT;
> +    Kind = BitMapKind;
> +    ++NumBitMaps;
> +    return;
> +  }
> +
>     // Store the table in an array.
>     ArrayType *ArrayTy = ArrayType::get(DefaultValue->getType(), TableSize);
>     Constant *Initializer = ConstantArray::get(ArrayTy, TableContents);
> @@ -3339,6 +3372,32 @@
>     switch (Kind) {
>       case SingleValueKind:
>         return SingleValue;
> +    case BitMapKind: {
> +      // Type of the bitmap (e.g. i59).
> +      IntegerType *MapTy = BitMap->getType();
> +
> +      // 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)?

Otherwise LGTM.

Ciao, Duncan.