[llvm] r222416 - SimplifyCFG: Refactor GatherConstantCompares() result in a struct

Timur Iskhodzhanov timurrrr at google.com
Thu Nov 20 03:51:06 PST 2014


On Thu Nov 20 2014 at 9:53:42 AM Mehdi Amini <mehdi.amini at apple.com> wrote:

> Author: mehdi_amini
> Date: Thu Nov 20 00:51:02 2014
> New Revision: 222416
>
> URL: http://llvm.org/viewvc/llvm-project?rev=222416&view=rev
> Log:
> SimplifyCFG: Refactor GatherConstantCompares() result in a struct
>
> Code seems cleaner and easier to understand this way
>
>
>
> 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=222416&r1=222415&r2=222416&view=diff
> ============================================================
> ==================
> --- llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp Thu Nov 20 00:51:02
> 2014
> @@ -357,160 +357,170 @@ static ConstantInt *GetConstantInt(Value
>    return nullptr;
>  }
>
> +/// Given a chain of or (||) or and (&&) comparison of a value against a
> +/// constant, this will try to recover the information required for a
> switch
> +/// structure.
> +/// It will depth-first traverse the chain of comparison, seeking for
> patterns
> +/// like %a == 12 or %a < 4 and combine them to produce a set of integer
> +/// representing the different cases for the switch.
> +/// Note that if the chain is composed of '||' it will build the set of
> elements
> +/// that matches the comparisons (i.e. any of this value validate the
> chain)
> +/// while for a chain of '&&' it will build the set elements that make
> the test
> +/// fail.
> +struct ConstantComparesGatherer {
> +
> +  Value *CompValue = nullptr; /// Value found for the switch comparison
> +  Value *Extra = nullptr;  /// Extra clause to be checked before the
> switch
> +  SmallVector<ConstantInt*, 8> Vals; /// Set of integers to match in
> switch
> +  unsigned UsedICmps = 0; /// Number of comparisons matched in the and/or
> chain
> +
> +  /// Construct and compute the result for the comparison instruction Cond
> +  ConstantComparesGatherer(Instruction *Cond, const DataLayout *DL) {
> +    gather(Cond, DL);
> +  }
> +
> +  /// Prevent copy
> +  ConstantComparesGatherer(const ConstantComparesGatherer&) = delete;
> +  ConstantComparesGatherer &operator=(const ConstantComparesGatherer&) =
> delete;
> +
> +private:
> +
> +  /// Try to set the current value used for the comparison, it succeeds
> only if
> +  /// it wasn't set before or if the new value is the same as the old one
> +  bool setValueOnce(Value *NewVal) {
> +    if(CompValue && CompValue != NewVal) return false;
> +    return CompValue = NewVal;
>

This looks like a typo.


> +  }
> +
>


> +  /// Try to match Instruction "I" as a comparison against a constant and
> +  /// populates the array Vals with the set of values that match (or do
> not
> +  /// match depending on isEQ).
> +  /// Return false on failure. On success, the Value the comparison
> matched
> +  /// against is placed in CompValue.
> +  /// If CompValue is already set, the function is expected to fail if a
> match
> +  /// is found but the value compared to is different.
> +  bool matchInstruction(Instruction *I, const DataLayout *DL, bool isEQ) {
> +    // If this is an icmp against a constant, handle this as one of the
> cases.
> +    ICmpInst *ICI;
> +    ConstantInt *C;
> +    if (!((ICI = dyn_cast<ICmpInst>(I)) &&
> +             (C = GetConstantInt(I->getOperand(1), DL)))) {
> +      return false;
> +    }
>
> +    Value *RHSVal;
> +    ConstantInt *RHSC;
>
> -// Try to match Instruction I as a comparison against a constant and
> populates
> -// Vals with the set of value that match (or does not depending on isEQ).
> -// Return nullptr on failure, or return the Value the comparison matched
> against
> -// on success
> -// CurrValue, if supplied, is the value we want to match against. The
> function
> -// is expected to fail if a match is found but the value compared to is
> not the
> -// one expected. If CurrValue is supplied, the return value has to be
> either
> -// nullptr or CurrValue
> -static Value* GatherConstantComparesMatch(Instruction *I,
> -                                          Value *CurrValue,
> -                                          SmallVectorImpl<ConstantInt*>
> &Vals,
> -                                          const DataLayout *DL,
> -                                          unsigned &UsedICmps,
> -                                          bool isEQ) {
> -
> -  // If this is an icmp against a constant, handle this as one of the
> cases.
> -  ICmpInst *ICI;
> -  ConstantInt *C;
> -  if (!((ICI = dyn_cast<ICmpInst>(I)) &&
> -        (C = GetConstantInt(I->getOperand(1), DL)))) {
> -    return nullptr;
> -  }
> -
> -  Value *RHSVal;
> -  ConstantInt *RHSC;
> -
> -  // Pattern match a special case
> -  // (x & ~2^x) == y --> x == y || x == y|2^x
> -  // This undoes a transformation done by instcombine to fuse 2 compares.
> -  if (ICI->getPredicate() == (isEQ ? ICmpInst::ICMP_EQ:ICmpInst::ICMP_NE))
> {
> -    if (match(ICI->getOperand(0),
> -              m_And(m_Value(RHSVal), m_ConstantInt(RHSC)))) {
> -      APInt Not = ~RHSC->getValue();
> -      if (Not.isPowerOf2()) {
> -        // If we already have a value for the switch, it has to match!
> -        if(CurrValue && CurrValue != RHSVal)
> -          return nullptr;
> -
> -        Vals.push_back(C);
> -        Vals.push_back(ConstantInt::get(C->getContext(),
> -                                        C->getValue() | Not));
> -        UsedICmps++;
> -        return RHSVal;
> +    // Pattern match a special case
> +    // (x & ~2^x) == y --> x == y || x == y|2^x
> +    // This undoes a transformation done by instcombine to fuse 2
> compares.
> +    if (ICI->getPredicate() == (isEQ ? ICmpInst::ICMP_EQ:ICmpInst::ICMP_NE))
> {
> +      if (match(ICI->getOperand(0),
> +                m_And(m_Value(RHSVal), m_ConstantInt(RHSC)))) {
> +        APInt Not = ~RHSC->getValue();
> +        if (Not.isPowerOf2()) {
> +          // If we already have a value for the switch, it has to match!
> +          if(!setValueOnce(RHSVal))
> +            return false;
> +
> +          Vals.push_back(C);
> +          Vals.push_back(ConstantInt::get(C->getContext(),
> +                                          C->getValue() | Not));
> +          UsedICmps++;
> +          return true;
> +        }
>        }
> -    }
>
> -    // If we already have a value for the switch, it has to match!
> -    if(CurrValue && CurrValue != ICI->getOperand(0))
> -      return nullptr;
> +      // If we already have a value for the switch, it has to match!
> +      if(!setValueOnce(ICI->getOperand(0)))
> +        return false;
>
> -    UsedICmps++;
> -    Vals.push_back(C);
> -    return ICI->getOperand(0);
> -  }
> +      UsedICmps++;
> +      Vals.push_back(C);
> +      return ICI->getOperand(0);
> +    }
> +
> +    // If we have "x ult 3", for example, then we can add 0,1,2 to the
> set.
> +    ConstantRange Span = ConstantRange::makeICmpRegion(
> ICI->getPredicate(),
> +                                                       C->getValue());
> +
> +    // Shift the range if the compare is fed by an add. This is the range
> +    // compare idiom as emitted by instcombine.
> +    Value *CandidateVal = I->getOperand(0);
> +    if(match(I->getOperand(0), m_Add(m_Value(RHSVal),
> m_ConstantInt(RHSC)))) {
> +      Span = Span.subtract(RHSC->getValue());
> +      CandidateVal = RHSVal;
> +    }
> +
> +    // If this is an and/!= check, then we are looking to build the set of
> +    // value that *don't* pass the and chain. I.e. to turn "x ugt 2" into
> +    // x != 0 && x != 1.
> +    if (!isEQ)
> +      Span = Span.inverse();
>
> -  // If we have "x ult 3", for example, then we can add 0,1,2 to the set.
> -  ConstantRange Span = ConstantRange::makeICmpRegion(ICI->getPredicate(),
> -                                                     C->getValue());
> +    // If there are a ton of values, we don't want to make a ginormous
> switch.
> +    if (Span.getSetSize().ugt(8) || Span.isEmptySet()) {
> +      return false;
> +    }
>
> -  // Shift the range if the compare is fed by an add. This is the range
> -  // compare idiom as emitted by instcombine.
> -  Value *CandidateVal = I->getOperand(0);
> -  if(match(I->getOperand(0), m_Add(m_Value(RHSVal),
> m_ConstantInt(RHSC)))) {
> -    Span = Span.subtract(RHSC->getValue());
> -    CandidateVal = RHSVal;
> -  }
> +    // If we already have a value for the switch, it has to match!
> +    if(!setValueOnce(CandidateVal))
> +      return false;
>
> -  // If we already have a value for the switch, it has to match!
> -  if(CurrValue && CurrValue != CandidateVal)
> -    return nullptr;
> +    // Add all values from the range to the set
> +    for (APInt Tmp = Span.getLower(); Tmp != Span.getUpper(); ++Tmp)
> +      Vals.push_back(ConstantInt::get(I->getContext(), Tmp));
>
> -  // If this is an and/!= check, then we are looking to build the set of
> -  // value that *don't* pass the and chain. I.e. to turn "x ugt 2" into
> -  // x != 0 && x != 1.
> -  if (!isEQ)
> -    Span = Span.inverse();
> +    UsedICmps++;
> +    return true;
>
> -  // If there are a ton of values, we don't want to make a ginormous
> switch.
> -  if (Span.getSetSize().ugt(8) || Span.isEmptySet()) {
> -    return nullptr;
>    }
>
> -  // Add all values from the range to the set
> -  for (APInt Tmp = Span.getLower(); Tmp != Span.getUpper(); ++Tmp)
> -    Vals.push_back(ConstantInt::get(I->getContext(), Tmp));
> -
> -  UsedICmps++;
> -  return CandidateVal;
> -
> -}
> -
> -/// GatherConstantCompares - Given a potentially 'or'd or 'and'd together
> -/// collection of icmp eq/ne instructions that compare a value against a
> -/// constant, return the value being compared, and stick the constant
> into the
> -/// Values vector.
> -/// One "Extra" case is allowed to differ from the other.
> -static Value *
> -GatherConstantCompares(Value *V, SmallVectorImpl<ConstantInt*> &Vals,
> Value *&Extra,
> -                       const DataLayout *DL, unsigned &UsedICmps) {
> -  Instruction *I = dyn_cast<Instruction>(V);
> -  if (!I) return nullptr;
> -
> -  bool isEQ = (I->getOpcode() == Instruction::Or);
> -
> -  // Keep a stack (SmallVector for efficiency) for depth-first traversal
> -  SmallVector<Value *, 8> DFT;
> -
> -  // Initialize
> -  DFT.push_back(V);
> -
> -  // Will hold the value used for the switch comparison
> -  Value *CurrValue = nullptr;
> -
> -  while(!DFT.empty()) {
> -    V = DFT.pop_back_val();
> -
> -    if (Instruction *I = dyn_cast<Instruction>(V)) {
> +  /// gather - Given a potentially 'or'd or 'and'd together collection of
> icmp
> +  /// eq/ne/lt/gt instructions that compare a value against a constant,
> extract
> +  /// the value being compared, and stick the list constants into the Vals
> +  /// vector.
> +  /// One "Extra" case is allowed to differ from the other.
> +  void gather(Value *V, const DataLayout *DL) {
> +    Instruction *I = dyn_cast<Instruction>(V);
> +    bool isEQ = (I->getOpcode() == Instruction::Or);
> +
> +    // Keep a stack (SmallVector for efficiency) for depth-first traversal
> +    SmallVector<Value *, 8> DFT;
> +
> +    // Initialize
> +    DFT.push_back(V);
> +
> +    while(!DFT.empty()) {
> +      V = DFT.pop_back_val();
> +
> +      if (Instruction *I = dyn_cast<Instruction>(V)) {
> +        // If it is a || (or && depending on isEQ), process the operands.
> +        if (I->getOpcode() == (isEQ ? Instruction::Or :
> Instruction::And)) {
> +          DFT.push_back(I->getOperand(1));
> +          DFT.push_back(I->getOperand(0));
> +          continue;
> +        }
>
> -      // If it is a || (or && depending on isEQ), process the operands.
> -      if (I->getOpcode() == (isEQ ? Instruction::Or : Instruction::And)) {
> -        DFT.push_back(I->getOperand(1));
> -        DFT.push_back(I->getOperand(0));
> -        continue;
> +        // Try to match the current instruction
> +        if (matchInstruction(I, DL, isEQ))
> +          // Match succeed, continue the loop
> +          continue;
>        }
>
> -      // Try to match the current instruction
> -      if (Value *Matched = GatherConstantComparesMatch(I,
> -                                                       CurrValue,
> -                                                       Vals,
> -                                                       DL,
> -                                                       UsedICmps,
> -                                                       isEQ)) {
> -        // Match succeed, continue the loop
> -        CurrValue = Matched;
> +      // One element of the sequence of || (or &&) could not be match as a
> +      // comparison against the same value as the others.
> +      // We allow only one "Extra" case to be checked before the switch
> +      if (!Extra) {
> +        Extra = V;
>          continue;
>        }
> +      // Failed to parse a proper sequence, abort now
> +      CompValue = nullptr;
> +      break;
>      }
> -
> -    // One element of the sequence of || (or &&) could not be match as a
> -    // comparison against the same value as the others.
> -    // We allow only one "Extra" case to be checked before the switch
> -    if (Extra == nullptr) {
> -      Extra = V;
> -      continue;
> -    }
> -    return nullptr;
> -
>    }
> -
> -  // Return the value to be used for the switch comparison (if any)
> -  return CurrValue;
> -}
> +};
>
>  static void EraseTerminatorInstAndDCECond(TerminatorInst *TI) {
>    Instruction *Cond = nullptr;
> @@ -2810,18 +2820,17 @@ static bool SimplifyBranchOnICmpChain(Br
>    Instruction *Cond = dyn_cast<Instruction>(BI->getCondition());
>    if (!Cond) return false;
>
> -
>    // Change br (X == 0 | X == 1), T, F into a switch instruction.
>    // If this is a bunch of seteq's or'd together, or if it's a bunch of
>    // 'setne's and'ed together, collect them.
> -  Value *CompVal = nullptr;
> -  SmallVector<ConstantInt*, 8> Values;
> -  bool TrueWhenEqual = (Cond->getOpcode() == Instruction::Or);
> -  Value *ExtraCase = nullptr;
> -  unsigned UsedICmps = 0;
>
>    // Try to gather values from a chain of and/or to be turned into a
> switch
> -  CompVal = GatherConstantCompares(Cond, Values, ExtraCase, DL,
> UsedICmps);
> +  ConstantComparesGatherer ConstantCompare{Cond, DL};
> +  // Unpack the result
> +  SmallVectorImpl<ConstantInt*> &Values = ConstantCompare.Vals;
> +  Value *CompVal = ConstantCompare.CompValue;
> +  unsigned UsedICmps = ConstantCompare.UsedICmps;
> +  Value *ExtraCase = ConstantCompare.Extra;
>
>    // If we didn't have a multiply compared value, fail.
>    if (!CompVal) return false;
> @@ -2830,6 +2839,8 @@ static bool SimplifyBranchOnICmpChain(Br
>    if (UsedICmps <= 1)
>      return false;
>
> +  bool TrueWhenEqual = (Cond->getOpcode() == Instruction::Or);
> +
>    // There might be duplicate constants in the list, which the switch
>    // instruction can't handle, remove them now.
>    array_pod_sort(Values.begin(), Values.end(), ConstantIntSortPredicate);
>
>
> _______________________________________________
> 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/20141120/725c4b9e/attachment.html>


More information about the llvm-commits mailing list