[llvm] r222935 - Simplify some more ownership using forward_list<T> rather than vector<unique_ptr<T>>

David Blaikie dblaikie at gmail.com
Fri Nov 28 13:39:07 PST 2014


On Fri, Nov 28, 2014 at 1:23 PM, Craig Topper <craig.topper at gmail.com>
wrote:

> Thanks for the cleanup!
>

Sure thing - and thank you too! (for all the cleanup, and getting the ball
rolling even when it's not the final state - incremental improvement is
great (& I wouldn't claim that my cleanups are necessarily the final state
either)) :)


>
> On Fri, Nov 28, 2014 at 3:20 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>> Author: dblaikie
>> Date: Fri Nov 28 15:20:24 2014
>> New Revision: 222935
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=222935&view=rev
>> Log:
>> Simplify some more ownership using forward_list<T> rather than
>> vector<unique_ptr<T>>
>>
>> Modified:
>>     llvm/trunk/utils/TableGen/AsmMatcherEmitter.cpp
>>
>> Modified: llvm/trunk/utils/TableGen/AsmMatcherEmitter.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/AsmMatcherEmitter.cpp?rev=222935&r1=222934&r2=222935&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/utils/TableGen/AsmMatcherEmitter.cpp (original)
>> +++ llvm/trunk/utils/TableGen/AsmMatcherEmitter.cpp Fri Nov 28 15:20:24
>> 2014
>> @@ -616,7 +616,7 @@ public:
>>    std::forward_list<ClassInfo> Classes;
>>
>>    /// The information on the matchables to match.
>> -  std::vector<std::unique_ptr<MatchableInfo>> Matchables;
>> +  std::forward_list<MatchableInfo> Matchables;
>>
>>    /// Info for custom matching operands by user defined methods.
>>    std::vector<OperandMatchEntry> OperandMatchInfo;
>> @@ -1285,8 +1285,8 @@ void AsmMatcherInfo::buildOperandMatchIn
>>
>>      // Keep track of all operands of this instructions which belong to
>> the
>>      // same class.
>> -    for (unsigned i = 0, e = MI->AsmOperands.size(); i != e; ++i) {
>> -      const MatchableInfo::AsmOperand &Op = MI->AsmOperands[i];
>> +    for (unsigned i = 0, e = MI.AsmOperands.size(); i != e; ++i) {
>> +      const MatchableInfo::AsmOperand &Op = MI.AsmOperands[i];
>>        if (Op.Class->ParserMethod.empty())
>>          continue;
>>        unsigned &OperandMask = OpClassMask[Op.Class];
>> @@ -1297,8 +1297,7 @@ void AsmMatcherInfo::buildOperandMatchIn
>>      for (const auto &OCM : OpClassMask) {
>>        unsigned OpMask = OCM.second;
>>        ClassInfo *CI = OCM.first;
>> -      OperandMatchInfo.push_back(OperandMatchEntry::create(MI.get(), CI,
>> -                                                           OpMask));
>> +      OperandMatchInfo.push_back(OperandMatchEntry::create(&MI, CI,
>> OpMask));
>>      }
>>    }
>>  }
>> @@ -1345,16 +1344,15 @@ void AsmMatcherInfo::buildInfo() {
>>        if (CGI->TheDef->getValueAsBit("isCodeGenOnly"))
>>          continue;
>>
>> -      std::unique_ptr<MatchableInfo> II(new MatchableInfo(*CGI));
>> +      Matchables.emplace_front(*CGI);
>> +      MatchableInfo *II = &Matchables.front();
>>
>>        II->initialize(*this, SingletonRegisters, AsmVariantNo,
>> RegisterPrefix);
>>
>>        // Ignore instructions which shouldn't be matched and diagnose
>> invalid
>>        // instruction definitions with an error.
>>        if (!II->validate(CommentDelimiter, true))
>> -        continue;
>> -
>> -      Matchables.push_back(std::move(II));
>> +        Matchables.pop_front();
>>      }
>>
>>      // Parse all of the InstAlias definitions and stick them in the list
>> of
>> @@ -1372,14 +1370,13 @@ void AsmMatcherInfo::buildInfo() {
>>              .startswith( MatchPrefix))
>>          continue;
>>
>> -      std::unique_ptr<MatchableInfo> II(new MatchableInfo(Alias));
>> +      Matchables.emplace_front(Alias);
>> +      MatchableInfo *II = &Matchables.front();
>>
>>        II->initialize(*this, SingletonRegisters, AsmVariantNo,
>> RegisterPrefix);
>>
>>        // Validate the alias definitions.
>>        II->validate(CommentDelimiter, false);
>> -
>> -      Matchables.push_back(std::move(II));
>>      }
>>    }
>>
>> @@ -1391,17 +1388,17 @@ void AsmMatcherInfo::buildInfo() {
>>
>>    // Build the information about matchables, now that we have fully
>> formed
>>    // classes.
>> -  std::vector<std::unique_ptr<MatchableInfo>> NewMatchables;
>> +  std::forward_list<MatchableInfo> NewMatchables;
>>    for (auto &II : Matchables) {
>>      // Parse the tokens after the mnemonic.
>>      // Note: buildInstructionOperandReference may insert new
>> AsmOperands, so
>>      // don't precompute the loop bound.
>> -    for (unsigned i = 0; i != II->AsmOperands.size(); ++i) {
>> -      MatchableInfo::AsmOperand &Op = II->AsmOperands[i];
>> +    for (unsigned i = 0; i != II.AsmOperands.size(); ++i) {
>> +      MatchableInfo::AsmOperand &Op = II.AsmOperands[i];
>>        StringRef Token = Op.Token;
>>
>>        // Check for singleton registers.
>> -      if (Record *RegRecord = II->AsmOperands[i].SingletonReg) {
>> +      if (Record *RegRecord = II.AsmOperands[i].SingletonReg) {
>>          Op.Class = RegisterClasses[RegRecord];
>>          assert(Op.Class && Op.Class->Registers.size() == 1 &&
>>                 "Unexpected class for singleton register");
>> @@ -1426,35 +1423,30 @@ void AsmMatcherInfo::buildInfo() {
>>        else
>>          OperandName = Token.substr(1);
>>
>> -      if (II->DefRec.is<const CodeGenInstruction*>())
>> -        buildInstructionOperandReference(II.get(), OperandName, i);
>> +      if (II.DefRec.is<const CodeGenInstruction *>())
>> +        buildInstructionOperandReference(&II, OperandName, i);
>>        else
>> -        buildAliasOperandReference(II.get(), OperandName, Op);
>> +        buildAliasOperandReference(&II, OperandName, Op);
>>      }
>>
>> -    if (II->DefRec.is<const CodeGenInstruction*>()) {
>> -      II->buildInstructionResultOperands();
>> +    if (II.DefRec.is<const CodeGenInstruction *>()) {
>> +      II.buildInstructionResultOperands();
>>        // If the instruction has a two-operand alias, build up the
>>        // matchable here. We'll add them in bulk at the end to avoid
>>        // confusing this loop.
>>        std::string Constraint =
>> -        II->TheDef->getValueAsString("TwoOperandAliasConstraint");
>> +          II.TheDef->getValueAsString("TwoOperandAliasConstraint");
>>        if (Constraint != "") {
>>          // Start by making a copy of the original matchable.
>> -        std::unique_ptr<MatchableInfo> AliasII(new MatchableInfo(*II));
>> +        NewMatchables.emplace_front(II);
>>
>>          // Adjust it to be a two-operand alias.
>> -        AliasII->formTwoOperandAlias(Constraint);
>> -
>> -        // Add the alias to the matchables list.
>> -        NewMatchables.push_back(std::move(AliasII));
>> +        NewMatchables.front().formTwoOperandAlias(Constraint);
>>        }
>>      } else
>> -      II->buildAliasResultOperands();
>> +      II.buildAliasResultOperands();
>>    }
>> -  if (!NewMatchables.empty())
>> -    std::move(NewMatchables.begin(), NewMatchables.end(),
>> -              std::back_inserter(Matchables));
>> +  Matchables.splice_after(Matchables.before_begin(), NewMatchables);
>>
>>    // Process token alias definitions and set up the associated superclass
>>    // information.
>> @@ -1679,9 +1671,8 @@ static unsigned getConverterOperandID(co
>>    return ID;
>>  }
>>
>> -
>>  static void emitConvertFuncs(CodeGenTarget &Target, StringRef ClassName,
>> -                             std::vector<std::unique_ptr<MatchableInfo>>
>> &Infos,
>> +                             std::forward_list<MatchableInfo> &Infos,
>>                               raw_ostream &OS) {
>>    SetVector<std::string> OperandConversionKinds;
>>    SetVector<std::string> InstructionConversionKinds;
>> @@ -1748,10 +1739,10 @@ static void emitConvertFuncs(CodeGenTarg
>>    for (auto &II : Infos) {
>>      // Check if we have a custom match function.
>>      std::string AsmMatchConverter =
>> -      II->getResultInst()->TheDef->getValueAsString("AsmMatchConverter");
>> +
>> II.getResultInst()->TheDef->getValueAsString("AsmMatchConverter");
>>      if (!AsmMatchConverter.empty()) {
>>        std::string Signature = "ConvertCustom_" + AsmMatchConverter;
>> -      II->ConversionFnKind = Signature;
>> +      II.ConversionFnKind = Signature;
>>
>>        // Check if we have already generated this signature.
>>        if (!InstructionConversionKinds.insert(Signature))
>> @@ -1783,17 +1774,17 @@ static void emitConvertFuncs(CodeGenTarg
>>      std::vector<uint8_t> ConversionRow;
>>
>>      // Compute the convert enum and the case body.
>> -    MaxRowLength = std::max(MaxRowLength, II->ResOperands.size()*2 + 1 );
>> +    MaxRowLength = std::max(MaxRowLength, II.ResOperands.size() * 2 + 1);
>>
>> -    for (unsigned i = 0, e = II->ResOperands.size(); i != e; ++i) {
>> -      const MatchableInfo::ResOperand &OpInfo = II->ResOperands[i];
>> +    for (unsigned i = 0, e = II.ResOperands.size(); i != e; ++i) {
>> +      const MatchableInfo::ResOperand &OpInfo = II.ResOperands[i];
>>
>>        // Generate code to populate each result operand.
>>        switch (OpInfo.Kind) {
>>        case MatchableInfo::ResOperand::RenderAsmOperand: {
>>          // This comes from something we parsed.
>>          const MatchableInfo::AsmOperand &Op =
>> -          II->AsmOperands[OpInfo.AsmOperandNum];
>> +            II.AsmOperands[OpInfo.AsmOperandNum];
>>
>>          // Registers are always converted the same, don't duplicate the
>>          // conversion function based on them.
>> @@ -1916,7 +1907,7 @@ static void emitConvertFuncs(CodeGenTarg
>>      if (Signature == "Convert")
>>        Signature += "_NoOperands";
>>
>> -    II->ConversionFnKind = Signature;
>> +    II.ConversionFnKind = Signature;
>>
>>      // Save the signature. If we already have it, don't add a new row
>>      // to the table.
>> @@ -2602,23 +2593,21 @@ void AsmMatcherEmitter::run(raw_ostream
>>    // Sort the instruction table using the partial order on classes. We
>> use
>>    // stable_sort to ensure that ambiguous instructions are still
>>    // deterministically ordered.
>> -  std::stable_sort(Info.Matchables.begin(), Info.Matchables.end(),
>> -                   [](const std::unique_ptr<MatchableInfo> &a,
>> -                      const std::unique_ptr<MatchableInfo> &b){
>> -                     return *a < *b;});
>> +  Info.Matchables.sort();
>>
>>    DEBUG_WITH_TYPE("instruction_info", {
>>        for (const auto &MI : Info.Matchables)
>> -        MI->dump();
>> +        MI.dump();
>>      });
>>
>>    // Check for ambiguous matchables.
>>    DEBUG_WITH_TYPE("ambiguous_instrs", {
>>      unsigned NumAmbiguous = 0;
>> -    for (unsigned i = 0, e = Info.Matchables.size(); i != e; ++i) {
>> -      for (unsigned j = i + 1; j != e; ++j) {
>> -        const MatchableInfo &A = *Info.Matchables[i];
>> -        const MatchableInfo &B = *Info.Matchables[j];
>> +    for (auto I = Info.Matchables.begin(), E = Info.Matchables.end(); I
>> != E;
>> +         ++I) {
>> +      for (auto J = std::next(I); J != E; ++J) {
>> +        const MatchableInfo &A = *I;
>> +        const MatchableInfo &B = *J;
>>
>>          if (A.couldMatchAmbiguouslyWith(B)) {
>>            errs() << "warning: ambiguous matchables:\n";
>> @@ -2739,11 +2728,11 @@ void AsmMatcherEmitter::run(raw_ostream
>>    unsigned MaxMnemonicIndex = 0;
>>    bool HasDeprecation = false;
>>    for (const auto &MI : Info.Matchables) {
>> -    MaxNumOperands = std::max(MaxNumOperands, MI->AsmOperands.size());
>> -    HasDeprecation |= MI->HasDeprecation;
>> +    MaxNumOperands = std::max(MaxNumOperands, MI.AsmOperands.size());
>> +    HasDeprecation |= MI.HasDeprecation;
>>
>>      // Store a pascal-style length byte in the mnemonic.
>> -    std::string LenMnemonic = char(MI->Mnemonic.size()) +
>> MI->Mnemonic.str();
>> +    std::string LenMnemonic = char(MI.Mnemonic.size()) +
>> MI.Mnemonic.str();
>>      MaxMnemonicIndex = std::max(MaxMnemonicIndex,
>>                          StringTable.GetOrAddStringOffset(LenMnemonic,
>> false));
>>    }
>> @@ -2767,8 +2756,9 @@ void AsmMatcherEmitter::run(raw_ostream
>>    OS << "    " << getMinimalTypeForRange(MaxMnemonicIndex)
>>                 << " Mnemonic;\n";
>>    OS << "    uint16_t Opcode;\n";
>> -  OS << "    " << getMinimalTypeForRange(Info.Matchables.size())
>> -               << " ConvertFn;\n";
>> +  OS << "    " <<
>> getMinimalTypeForRange(std::distance(Info.Matchables.begin(),
>> +
>>  Info.Matchables.end()))
>> +     << " ConvertFn;\n";
>>    OS << "    " << getMinimalRequiredFeaturesType(Info)
>>                 << " RequiredFeatures;\n";
>>    OS << "    " << getMinimalTypeForRange(
>> @@ -2803,29 +2793,28 @@ void AsmMatcherEmitter::run(raw_ostream
>>      OS << "static const MatchEntry MatchTable" << VC << "[] = {\n";
>>
>>      for (const auto &MI : Info.Matchables) {
>> -      if (MI->AsmVariantID != AsmVariantNo)
>> +      if (MI.AsmVariantID != AsmVariantNo)
>>          continue;
>>
>>        // Store a pascal-style length byte in the mnemonic.
>> -      std::string LenMnemonic = char(MI->Mnemonic.size()) +
>> MI->Mnemonic.str();
>> +      std::string LenMnemonic = char(MI.Mnemonic.size()) +
>> MI.Mnemonic.str();
>>        OS << "  { " << StringTable.GetOrAddStringOffset(LenMnemonic,
>> false)
>> -         << " /* " << MI->Mnemonic << " */, "
>> -         << Target.getName() << "::"
>> -         << MI->getResultInst()->TheDef->getName() << ", "
>> -         << MI->ConversionFnKind << ", ";
>> +         << " /* " << MI.Mnemonic << " */, " << Target.getName()
>> +         << "::" << MI.getResultInst()->TheDef->getName() << ", "
>> +         << MI.ConversionFnKind << ", ";
>>
>>        // Write the required features mask.
>> -      if (!MI->RequiredFeatures.empty()) {
>> -        for (unsigned i = 0, e = MI->RequiredFeatures.size(); i != e;
>> ++i) {
>> +      if (!MI.RequiredFeatures.empty()) {
>> +        for (unsigned i = 0, e = MI.RequiredFeatures.size(); i != e;
>> ++i) {
>>            if (i) OS << "|";
>> -          OS << MI->RequiredFeatures[i]->getEnumName();
>> +          OS << MI.RequiredFeatures[i]->getEnumName();
>>          }
>>        } else
>>          OS << "0";
>>
>>        OS << ", { ";
>> -      for (unsigned i = 0, e = MI->AsmOperands.size(); i != e; ++i) {
>> -        const MatchableInfo::AsmOperand &Op = MI->AsmOperands[i];
>> +      for (unsigned i = 0, e = MI.AsmOperands.size(); i != e; ++i) {
>> +        const MatchableInfo::AsmOperand &Op = MI.AsmOperands[i];
>>
>>          if (i) OS << ", ";
>>          OS << Op.Class->Name;
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
>
>
> --
> ~Craig
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141128/dec2fa89/attachment.html>


More information about the llvm-commits mailing list