[llvm] r213815 - SimplifyCFG: fix a bug in switch to table conversion
Hans Wennborg
hans at chromium.org
Thu Jul 24 10:23:46 PDT 2014
On Thu, Jul 24, 2014 at 8:16 AM, Manman <manman.ren at gmail.com> wrote:
> Sorry for the breakage. It seems simplifycfg (switch to table conversion) depends on the target.
Yes, the switch to table conversion only runs on targets that opt in
to it via TargetTransformInfo.
> I will update the testing case.
You can just move it to test/Transforms/SimplifyCFG/X86/ where the
other switch to table conversion tests live.
- Hans
>> On Jul 24, 2014, at 5:56 AM, Tilmann Scheller <t.scheller at samsung.com> wrote:
>>
>> Hi Manman,
>>
>> seems like this commit broke the clang-native-arm-cortex-a9 buildbot, can
>> you please have a look?
>>
>> http://lab.llvm.org:8011/builders/clang-native-arm-cortex-a9/builds/20308
>>
>> Regards,
>>
>> Tilmann
>>
>> -----Original Message-----
>> From: llvm-commits-bounces at cs.uiuc.edu
>> [mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of Manman Ren
>> Sent: Thursday, July 24, 2014 1:13 AM
>> To: llvm-commits at cs.uiuc.edu
>> 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/Simplify
>> CFG.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/s
>> witch-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
>>
>>
>
> _______________________________________________
> 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