[llvm] r222872 - Peephole optimization in switch table lookup: reuse the guarding table comparison if possible.

Evgeniy Stepanov eugeni.stepanov at gmail.com
Thu Nov 27 02:41:02 PST 2014


Hi,

this is breaking clang bootstrap. Please fix or revert!

http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/14293/steps/bootstrap%20clang/logs/stdio
http://lab.llvm.org:8011/builders/clang-native-aarch64/builds/1332
http://bb.pgr.jp/builders/clang-3stage-i686-linux/builds/980

On Thu, Nov 27, 2014 at 11:33 AM, Erik Eckstein <eeckstein at apple.com> wrote:
> Author: eeckstein
> Date: Thu Nov 27 02:33:51 2014
> New Revision: 222872
>
> URL: http://llvm.org/viewvc/llvm-project?rev=222872&view=rev
> Log:
> Peephole optimization in switch table lookup: reuse the guarding table comparison if possible.
>
> This optimization tries to reuse the generated compare instruction, if there is a comparison against the default value after the switch.
> Example:
>     if (idx < tablesize)
>        r = table[idx]; // table does not contain default_value
>     else
>        r = default_value;
>     if (r != default_value)
>        ...
> Is optimized to:
>     cond = idx < tablesize;
>     if (cond)
>        r = table[idx];
>     else
>        r = default_value;
>     if (cond)
>        ...
> \endcode
> Jump threading will then eliminate the second if(cond).
>
>
> Modified:
>     llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
>     llvm/trunk/test/Transforms/SimplifyCFG/X86/switch_to_lookup_table.ll
>
> Modified: llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp?rev=222872&r1=222871&r2=222872&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp Thu Nov 27 02:33:51 2014
> @@ -73,6 +73,7 @@ STATISTIC(NumBitMaps, "Number of switch
>  STATISTIC(NumLinearMaps, "Number of switch instructions turned into linear mapping");
>  STATISTIC(NumLookupTables, "Number of switch instructions turned into lookup tables");
>  STATISTIC(NumLookupTablesHoles, "Number of switch instructions turned into lookup tables (holes checked)");
> +STATISTIC(NumTableCmpReuses, "Number of reused switch table lookup compares");
>  STATISTIC(NumSinkCommons, "Number of common instructions sunk down to the end block");
>  STATISTIC(NumSpeculations, "Number of speculative executed instructions");
>
> @@ -3982,6 +3983,78 @@ static bool ShouldBuildLookupTable(Switc
>    return SI->getNumCases() * 10 >= TableSize * 4;
>  }
>
> +/// Try to reuse the switch table index compare. Following pattern:
> +/// \code
> +///     if (idx < tablesize)
> +///        r = table[idx]; // table does not contain default_value
> +///     else
> +///        r = default_value;
> +///     if (r != default_value)
> +///        ...
> +/// \endcode
> +/// Is optimized to:
> +/// \code
> +///     cond = idx < tablesize;
> +///     if (cond)
> +///        r = table[idx];
> +///     else
> +///        r = default_value;
> +///     if (cond)
> +///        ...
> +/// \endcode
> +/// Jump threading will then eliminate the second if(cond).
> +static void reuseTableCompare(User *PhiUser, BasicBlock *PhiBlock,
> +          BranchInst *RangeCheckBranch, Constant *DefaultValue,
> +          const SmallVectorImpl<std::pair<ConstantInt*, Constant*> >& Values) {
> +
> +  ICmpInst *CmpInst = dyn_cast<ICmpInst>(PhiUser);
> +  if (!CmpInst)
> +    return;
> +
> +  // We require that the compare is in the same block as the phi so that jump
> +  // threading can do its work afterwards.
> +  if (CmpInst->getParent() != PhiBlock)
> +    return;
> +
> +  Constant *CmpOp1 = dyn_cast<Constant>(CmpInst->getOperand(1));
> +  if (!CmpOp1)
> +    return;
> +
> +  Value *RangeCmp = RangeCheckBranch->getCondition();
> +  Constant *TrueConst = ConstantInt::getTrue(RangeCmp->getType());
> +  Constant *FalseConst = ConstantInt::getFalse(RangeCmp->getType());
> +
> +  // Check if the compare with the default value is constant true or false.
> +  Constant *DefaultConst = ConstantExpr::getICmp(CmpInst->getPredicate(),
> +                                                 DefaultValue, CmpOp1, true);
> +  if (DefaultConst != TrueConst && DefaultConst != FalseConst)
> +    return;
> +
> +  // Check if the compare with the case values is distinct from the default
> +  // compare result.
> +  for (auto ValuePair : Values) {
> +    Constant *CaseConst = ConstantExpr::getICmp(CmpInst->getPredicate(),
> +                              ValuePair.second, CmpOp1, true);
> +    if (!CaseConst || CaseConst == DefaultConst)
> +      return;
> +    assert((CaseConst == TrueConst || CaseConst == FalseConst) &&
> +           "Expect true or false as compare result.");
> +  }
> +
> +  if (DefaultConst == FalseConst) {
> +    // The compare yields the same result. We can replace it.
> +    CmpInst->replaceAllUsesWith(RangeCmp);
> +    ++NumTableCmpReuses;
> +  } else {
> +    // The compare yields the same result, just inverted. We can replace it.
> +    Value *InvertedTableCmp = BinaryOperator::CreateXor(RangeCmp,
> +                ConstantInt::get(RangeCmp->getType(), 1), "inverted.cmp",
> +                RangeCheckBranch);
> +    CmpInst->replaceAllUsesWith(InvertedTableCmp);
> +    ++NumTableCmpReuses;
> +  }
> +}
> +
>  /// SwitchToLookupTable - If the switch is only used to initialize one or more
>  /// phi nodes in a common successor block with different constant values,
>  /// replace the switch with lookup tables.
> @@ -4058,11 +4131,8 @@ static bool SwitchToLookupTable(SwitchIn
>    // If the table has holes, we need a constant result for the default case
>    // or a bitmask that fits in a register.
>    SmallVector<std::pair<PHINode*, Constant*>, 4> DefaultResultsList;
> -  bool HasDefaultResults = false;
> -  if (TableHasHoles) {
> -    HasDefaultResults = GetCaseResults(SI, nullptr, SI->getDefaultDest(),
> +  bool HasDefaultResults = GetCaseResults(SI, nullptr, SI->getDefaultDest(),
>                                         &CommonDest, DefaultResultsList, DL);
> -  }
>
>    bool NeedMask = (TableHasHoles && !HasDefaultResults);
>    if (NeedMask) {
> @@ -4106,6 +4176,8 @@ static bool SwitchToLookupTable(SwitchIn
>    // lookup table BB. Otherwise, check if the condition value is within the case
>    // range. If it is so, branch to the new BB. Otherwise branch to SI's default
>    // destination.
> +  BranchInst *RangeCheckBranch = nullptr;
> +
>    const bool GeneratingCoveredLookupTable = MaxTableSize == TableSize;
>    if (GeneratingCoveredLookupTable) {
>      Builder.CreateBr(LookupBB);
> @@ -4116,7 +4188,7 @@ static bool SwitchToLookupTable(SwitchIn
>    } else {
>      Value *Cmp = Builder.CreateICmpULT(TableIndex, ConstantInt::get(
>                                         MinCaseVal->getType(), TableSize));
> -    Builder.CreateCondBr(Cmp, LookupBB, SI->getDefaultDest());
> +    RangeCheckBranch = Builder.CreateCondBr(Cmp, LookupBB, SI->getDefaultDest());
>    }
>
>    // Populate the BB that does the lookups.
> @@ -4167,11 +4239,11 @@ static bool SwitchToLookupTable(SwitchIn
>    bool ReturnedEarly = false;
>    for (size_t I = 0, E = PHIs.size(); I != E; ++I) {
>      PHINode *PHI = PHIs[I];
> +    const ResultListTy &ResultList = ResultLists[PHI];
>
>      // If using a bitmask, use any value to fill the lookup table holes.
>      Constant *DV = NeedMask ? ResultLists[PHI][0].second : DefaultResults[PHI];
> -    SwitchLookupTable Table(Mod, TableSize, MinCaseVal, ResultLists[PHI],
> -                            DV, DL);
> +    SwitchLookupTable Table(Mod, TableSize, MinCaseVal, ResultList, DV, DL);
>
>      Value *Result = Table.BuildLookup(TableIndex, Builder);
>
> @@ -4184,6 +4256,16 @@ static bool SwitchToLookupTable(SwitchIn
>        break;
>      }
>
> +    // Do a small peephole optimization: re-use the switch table compare if
> +    // possible.
> +    if (!TableHasHoles && HasDefaultResults && RangeCheckBranch) {
> +      BasicBlock *PhiBlock = PHI->getParent();
> +      // Search for compare instructions which use the phi.
> +      for (auto *User : PHI->users()) {
> +        reuseTableCompare(User, PhiBlock, RangeCheckBranch, DV, ResultList);
> +      }
> +    }
> +
>      PHI->addIncoming(Result, LookupBB);
>    }
>
>
> Modified: llvm/trunk/test/Transforms/SimplifyCFG/X86/switch_to_lookup_table.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SimplifyCFG/X86/switch_to_lookup_table.ll?rev=222872&r1=222871&r2=222872&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/SimplifyCFG/X86/switch_to_lookup_table.ll (original)
> +++ llvm/trunk/test/Transforms/SimplifyCFG/X86/switch_to_lookup_table.ll Thu Nov 27 02:33:51 2014
> @@ -1077,3 +1077,97 @@ return:
>  ; CHECK-NEXT: ret i8 %switch.idx.cast
>  }
>
> +; Reuse the inverted table range compare.
> +define i32 @reuse_cmp1(i32 %x) {
> +entry:
> +  switch i32 %x, label %sw.default [
> +    i32 0, label %sw.bb
> +    i32 1, label %sw.bb1
> +    i32 2, label %sw.bb2
> +    i32 3, label %sw.bb3
> +  ]
> +sw.bb: br label %sw.epilog
> +sw.bb1: br label %sw.epilog
> +sw.bb2: br label %sw.epilog
> +sw.bb3: br label %sw.epilog
> +sw.default: br label %sw.epilog
> +sw.epilog:
> +  %r.0 = phi i32 [ 0, %sw.default ], [ 13, %sw.bb3 ], [ 12, %sw.bb2 ], [ 11, %sw.bb1 ], [ 10, %sw.bb ]
> +  %cmp = icmp eq i32 %r.0, 0       ; This compare can be "replaced".
> +  br i1 %cmp, label %if.then, label %if.end
> +if.then: br label %return
> +if.end: br label %return
> +return:
> +  %retval.0 = phi i32 [ 100, %if.then ], [ %r.0, %if.end ]
> +  ret i32 %retval.0
> +; CHECK-LABEL: @reuse_cmp1(
> +; CHECK: entry:
> +; CHECK-NEXT: %switch.tableidx = sub i32 %x, 0
> +; CHECK-NEXT: [[C:%.+]] = icmp ult i32 %switch.tableidx, 4
> +; CHECK-NEXT: %inverted.cmp = xor i1 [[C]], true
> +; CHECK:      [[R:%.+]] = select i1 %inverted.cmp, i32 100, i32 {{.*}}
> +; CHECK-NEXT: ret i32 [[R]]
> +}
> +
> +; Reuse the table range compare.
> +define i32 @reuse_cmp2(i32 %x) {
> +entry:
> +  switch i32 %x, label %sw.default [
> +    i32 0, label %sw.bb
> +    i32 1, label %sw.bb1
> +    i32 2, label %sw.bb2
> +    i32 3, label %sw.bb3
> +  ]
> +sw.bb: br label %sw.epilog
> +sw.bb1: br label %sw.epilog
> +sw.bb2: br label %sw.epilog
> +sw.bb3: br label %sw.epilog
> +sw.default: br label %sw.epilog
> +sw.epilog:
> +  %r.0 = phi i32 [ 4, %sw.default ], [ 3, %sw.bb3 ], [ 2, %sw.bb2 ], [ 1, %sw.bb1 ], [ 0, %sw.bb ]
> +  %cmp = icmp ne i32 %r.0, 4       ; This compare can be "replaced".
> +  br i1 %cmp, label %if.then, label %if.end
> +if.then: br label %return
> +if.end: br label %return
> +return:
> +  %retval.0 = phi i32 [ %r.0, %if.then ], [ 100, %if.end ]
> +  ret i32 %retval.0
> +; CHECK-LABEL: @reuse_cmp2(
> +; CHECK: entry:
> +; CHECK-NEXT: %switch.tableidx = sub i32 %x, 0
> +; CHECK-NEXT: [[C:%.+]] = icmp ult i32 %switch.tableidx, 4
> +; CHECK:      [[R:%.+]] = select i1 [[C]], i32 {{.*}}, i32 100
> +; CHECK-NEXT: ret i32 [[R]]
> +}
> +
> +; Cannot reuse the table range compare, because the default value is the same
> +; as one of the case values.
> +define i32 @no_reuse_cmp(i32 %x) {
> +entry:
> +  switch i32 %x, label %sw.default [
> +    i32 0, label %sw.bb
> +    i32 1, label %sw.bb1
> +    i32 2, label %sw.bb2
> +    i32 3, label %sw.bb3
> +  ]
> +sw.bb: br label %sw.epilog
> +sw.bb1: br label %sw.epilog
> +sw.bb2: br label %sw.epilog
> +sw.bb3: br label %sw.epilog
> +sw.default: br label %sw.epilog
> +sw.epilog:
> +  %r.0 = phi i32 [ 12, %sw.default ], [ 13, %sw.bb3 ], [ 12, %sw.bb2 ], [ 11, %sw.bb1 ], [ 10, %sw.bb ]
> +  %cmp = icmp ne i32 %r.0, 0
> +  br i1 %cmp, label %if.then, label %if.end
> +if.then: br label %return
> +if.end: br label %return
> +return:
> +  %retval.0 = phi i32 [ %r.0, %if.then ], [ 100, %if.end ]
> +  ret i32 %retval.0
> +; CHECK-LABEL: @no_reuse_cmp(
> +; CHECK:  [[S:%.+]] = select
> +; CHECK-NEXT:  %cmp = icmp ne i32 [[S]], 0
> +; CHECK-NEXT:  [[R:%.+]] = select i1 %cmp, i32 [[S]], i32 100
> +; CHECK-NEXT:  ret i32 [[R]]
> +}
> +
>
>
> _______________________________________________
> 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