[llvm] r213815 - SimplifyCFG: fix a bug in switch to table conversion
Hal Finkel
hfinkel at anl.gov
Thu Jul 24 05:31:51 PDT 2014
----- Original Message -----
> From: "Manman Ren" <manman.ren at gmail.com>
> To: llvm-commits at cs.uiuc.edu
> Sent: Wednesday, July 23, 2014 6:13:23 PM
> Subject: [llvm] r213815 - SimplifyCFG: fix a bug in switch to table conversion
>
> 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)))
FWIW, GCC warns here:
SimplifyCFG.cpp:3630:54: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
if (TableSize > (1 << (IT->getBitWidth() - 1)))
^
-Hal
> + 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
>
--
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory
More information about the llvm-commits
mailing list