[llvm] r213815 - SimplifyCFG: fix a bug in switch to table conversion

[Hans Wennborg]

On Wed, Jul 23, 2014 at 4:13 PM, Manman Ren <manman.ren at gmail.com> wrote:
> Author: mren
> Date: Wed Jul 23 18:13:23 2014
> New Revision: 213815
>
> URL: http://llvm.org/viewvc/llvm-project?rev=213815&view=rev
> Log:
> SimplifyCFG: fix a bug in switch to table conversion
>
> We use gep to access the global array "switch.table", and the table index
> should be treated as unsigned. When the highest bit is 1, this commit
> zero-extends the index to an integer type with larger size.

Thanks for catching and fixing this! Maybe it should get merged to the
3.5 branch?

The patch looks good; I only have a few suggestions inline below.

> -Value *SwitchLookupTable::BuildLookup(Value *Index, IRBuilder<> &Builder) {
> +Value *SwitchLookupTable::BuildLookup(Value *Index, uint64_t TableSize,
> +                                      IRBuilder<> &Builder) {

It feels wrong having to pass the TableSize here. SwitchLookupTable
should know its own size.

>    switch (Kind) {
>      case SingleValueKind:
>        return SingleValue;
> @@ -3624,6 +3625,14 @@ Value *SwitchLookupTable::BuildLookup(Va
>                                   "switch.masked");
>      }
>      case ArrayKind: {

We can get the table size from the Array member:

uint64_t TableSize =
cast<Constant>(Array->getInitializer())->getType()->getArrayNumElements();

(Or we could even store it as a new ArraySize member when we construct
the Array in the SwitchLookupTable constructor.)

> +      // Make sure the table index will not overflow when treated as signed.
> +      if (IntegerType *IT = dyn_cast<IntegerType>(Index->getType()))

The index is always of integer type, so no need for dyn_cast and if here.

Cheers,
Hans