SimplifyCFG: fix a bug in switch to table conversion
authorManman Ren <manman.ren@gmail.com>
Wed, 23 Jul 2014 23:13:23 +0000 (23:13 +0000)
committerManman Ren <manman.ren@gmail.com>
Wed, 23 Jul 2014 23:13:23 +0000 (23:13 +0000)
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
test/Transforms/SimplifyCFG/switch-table-bug.ll [new file with mode: 0644]

index 960b198f36bdf93c16a1b0cc57aa0e64b3b8a8f9..635117ff0b98c5b7e268daf1c35efb7cca772a7f 100644 (file)
@@ -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<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(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 (file)
index 0000000..d2e1907
--- /dev/null
@@ -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
+}