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

[Hans Wennborg]

Hi Bill,

Can we merge this and the follow-ups (r213880, 213884, 213895) to 3.5, please?

Thanks,
Hans

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.
>
> For a switch on i2, we used to generate:
> %switch.tableidx = sub i2 %0, -2
> getelementptr inbounds [4 x i64]* @switch.table, i32 0, i2 %switch.tableidx
>
> It is incorrect when %switch.tableidx is 2 or 3. The fix is to generate
> %switch.tableidx = sub i2 %0, -2
> %switch.tableidx.zext = zext i2 %switch.tableidx to i3
> getelementptr inbounds [4 x i64]* @switch.table, i32 0, i3 %switch.tableidx.zext
>
> rdar://17735071
>
> Added:
>     llvm/trunk/test/Transforms/SimplifyCFG/switch-table-bug.ll
> Modified:
>     llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
>
> Modified: llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp?rev=213815&r1=213814&r2=213815&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp Wed Jul 23 18:13:23 2014
> @@ -3477,7 +3477,7 @@ namespace {
>
>      /// BuildLookup - Build instructions with Builder to retrieve the value at
>      /// the position given by Index in the lookup table.
> -    Value *BuildLookup(Value *Index, IRBuilder<> &Builder);
> +    Value *BuildLookup(Value *Index, uint64_t TableSize, IRBuilder<> &Builder);
>
>      /// WouldFitInRegister - Return true if a table with TableSize elements of
>      /// type ElementType would fit in a target-legal register.
> @@ -3598,7 +3598,8 @@ SwitchLookupTable::SwitchLookupTable(Mod
>    Kind = ArrayKind;
>  }
>
> -Value *SwitchLookupTable::BuildLookup(Value *Index, IRBuilder<> &Builder) {
> +Value *SwitchLookupTable::BuildLookup(Value *Index, uint64_t TableSize,
> +                                      IRBuilder<> &Builder) {
>    switch (Kind) {
>      case SingleValueKind:
>        return SingleValue;
> @@ -3624,6 +3625,14 @@ Value *SwitchLookupTable::BuildLookup(Va
>                                   "switch.masked");
>      }
>      case ArrayKind: {
> +      // Make sure the table index will not overflow when treated as signed.
> +      if (IntegerType *IT = dyn_cast<IntegerType>(Index->getType()))
> +        if (TableSize > (1 << (IT->getBitWidth() - 1)))
> +          Index = Builder.CreateZExt(Index,
> +                                     IntegerType::get(IT->getContext(),
> +                                                      IT->getBitWidth() + 1),
> +                                     "switch.tableidx.zext");
> +
>        Value *GEPIndices[] = { Builder.getInt32(0), Index };
>        Value *GEP = Builder.CreateInBoundsGEP(Array, GEPIndices,
>                                               "switch.gep");
> @@ -3823,7 +3832,7 @@ static bool SwitchToLookupTable(SwitchIn
>      SI->getDefaultDest()->removePredecessor(SI->getParent());
>    } else {
>      Value *Cmp = Builder.CreateICmpULT(TableIndex, ConstantInt::get(
> -                                         MinCaseVal->getType(), TableSize));
> +                                       MinCaseVal->getType(), TableSize));
>      Builder.CreateCondBr(Cmp, LookupBB, SI->getDefaultDest());
>    }
>
> @@ -3878,7 +3887,7 @@ static bool SwitchToLookupTable(SwitchIn
>      SwitchLookupTable Table(Mod, TableSize, MinCaseVal, ResultLists[PHI],
>                              DV, DL);
>
> -    Value *Result = Table.BuildLookup(TableIndex, Builder);
> +    Value *Result = Table.BuildLookup(TableIndex, TableSize, Builder);
>
>      // If the result is used to return immediately from the function, we want to
>      // do that right here.
>
> Added: llvm/trunk/test/Transforms/SimplifyCFG/switch-table-bug.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SimplifyCFG/switch-table-bug.ll?rev=213815&view=auto
> ==============================================================================
> --- llvm/trunk/test/Transforms/SimplifyCFG/switch-table-bug.ll (added)
> +++ llvm/trunk/test/Transforms/SimplifyCFG/switch-table-bug.ll Wed Jul 23 18:13:23 2014
> @@ -0,0 +1,41 @@
> +; RUN: opt -S -simplifycfg < %s | FileCheck %s
> +; rdar://17735071
> +target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
> +target triple = "x86_64-apple-darwin12.0.0"
> +
> +; When tableindex can't fit into i2, we should extend the type to i3.
> +; CHECK-LABEL: @_TFO6reduce1E5toRawfS0_FT_Si
> +; CHECK: entry:
> +; CHECK-NEXT: sub i2 %0, -2
> +; CHECK-NEXT: zext i2 %switch.tableidx to i3
> +; CHECK-NEXT: getelementptr inbounds [4 x i64]* @switch.table, i32 0, i3 %switch.tableidx.zext
> +; CHECK-NEXT: load i64* %switch.gep
> +; CHECK-NEXT: ret i64 %switch.load
> +define i64 @_TFO6reduce1E5toRawfS0_FT_Si(i2) {
> +entry:
> +  switch i2 %0, label %1 [
> +    i2 0, label %2
> +    i2 1, label %3
> +    i2 -2, label %4
> +    i2 -1, label %5
> +  ]
> +
> +; <label>:1                                       ; preds = %entry
> +  unreachable
> +
> +; <label>:2                                       ; preds = %2
> +  br label %6
> +
> +; <label>:3                                       ; preds = %4
> +  br label %6
> +
> +; <label>:4                                       ; preds = %6
> +  br label %6
> +
> +; <label>:5                                       ; preds = %8
> +  br label %6
> +
> +; <label>:6                                      ; preds = %3, %5, %7, %9
> +  %7 = phi i64 [ 3, %5 ], [ 2, %4 ], [ 1, %3 ], [ 0, %2 ]
> +  ret i64 %7
> +}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits