[llvm] r222416 - SimplifyCFG: Refactor GatherConstantCompares() result in a struct
Timur Iskhodzhanov
timurrrr at google.com
Thu Nov 20 04:37:29 PST 2014
Reverted in r222428 along with r222426 and r222422
On Thu Nov 20 2014 at 3:08:48 PM Timur Iskhodzhanov <timurrrr at google.com>
wrote:
> Fixing this typo has resulted in buildbot errors, so I'm going to revert
> both mine and your changes [assuming you're not immediately available for a
> fix].
> http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/20839
>
> On Thu Nov 20 2014 at 2:51:06 PM Timur Iskhodzhanov <timurrrr at google.com>
> wrote:
>
>> 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/Transform
>>> s/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/c387f9ee/attachment.html>
More information about the llvm-commits
mailing list