From 4cae9cb0345162984c45da7769c5bfd2c62f6ff3 Mon Sep 17 00:00:00 2001 From: Manman Ren Date: Wed, 23 Jul 2014 23:13:23 +0000 Subject: [PATCH] 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 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@213815 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Transforms/Utils/SimplifyCFG.cpp | 17 ++++++-- .../SimplifyCFG/switch-table-bug.ll | 41 +++++++++++++++++++ 2 files changed, 54 insertions(+), 4 deletions(-) create mode 100644 test/Transforms/SimplifyCFG/switch-table-bug.ll diff --git a/lib/Transforms/Utils/SimplifyCFG.cpp b/lib/Transforms/Utils/SimplifyCFG.cpp index 960b198f36b..635117ff0b9 100644 --- a/lib/Transforms/Utils/SimplifyCFG.cpp +++ b/lib/Transforms/Utils/SimplifyCFG.cpp @@ -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(Module &M, 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(Value *Index, IRBuilder<> &Builder) { "switch.masked"); } case ArrayKind: { + // Make sure the table index will not overflow when treated as signed. + if (IntegerType *IT = dyn_cast(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(SwitchInst *SI, 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(SwitchInst *SI, 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. diff --git a/test/Transforms/SimplifyCFG/switch-table-bug.ll b/test/Transforms/SimplifyCFG/switch-table-bug.ll new file mode 100644 index 00000000000..d2e19075935 --- /dev/null +++ b/test/Transforms/SimplifyCFG/switch-table-bug.ll @@ -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 + ] + +;