[llvm] r213815 - SimplifyCFG: fix a bug in switch to table conversion
Hans Wennborg
hans at chromium.org
Tue Jul 29 16:54:05 PDT 2014
On Thu, Jul 24, 2014 at 5:14 PM, Hans Wennborg <hans at chromium.org> wrote:
> Hi Bill,
>
> Can we merge this and the follow-ups (r213880, 213884, 213895) to 3.5, please?
Merged in 214251, 214252, 214253, and 214254 respectively.
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
More information about the llvm-commits
mailing list