[llvm] r213815 - SimplifyCFG: fix a bug in switch to table conversion
Manman Ren
manman.ren at gmail.com
Thu Jul 24 10:28:43 PDT 2014
Thanks Hans for the tip!
I was following CoveredLookupTable.ll. I checked in r213880. If that does
not work, I will move it to X86.
Thanks,
Manman
On Thu, Jul 24, 2014 at 10:23 AM, Hans Wennborg <hans at chromium.org> wrote:
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140724/d4abf556/attachment.html>
More information about the llvm-commits
mailing list