[llvm] r222935 - Simplify some more ownership using forward_list<T> rather than vector<unique_ptr<T>>
Eric Christopher
echristo at gmail.com
Wed Dec 3 14:58:30 PST 2014
Mmm.. I have a build set up on darwin, I can take a look as well.
-eric
On Wed Dec 03 2014 at 2:42:49 PM David Blaikie <dblaikie at gmail.com> wrote:
> On Fri, Nov 28, 2014 at 3:04 PM, Duncan P. N. Exon Smith <
> dexonsmith at apple.com> wrote:
>
>> I reverted this (and the follow-up in r222938) in r222943.
>>
>> Skimming the commit, I suspect the failures are caused by front-loading
>> the sequence instead of back-loading, but I didn't have a chance to
>> investigate.
>>
>> Let me know if the failures don't reproduce for you and I can help you
>> dig into it.
>>
>
> Wouldn't mind a little help here - some of the tests failing (just taking
> the first one as a random example, test/CodeGen/X86/compact-unwind.ll)
> have hardcoded triples for darwin, so it doesn't seem like it's a matter of
> the target, so much as the host - I don't have a build setup on macos, so
> I'm not sure if there's anything more I can do to reproduce these
> failures...
>
>
> - David
>
>
>> > On 2014 Nov 28, at 14:38, Duncan P. N. Exon Smith <dexonsmith at apple.com>
>> wrote:
>> >
>> > Our public incremental builder started failing with this commit:
>> >
>> http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA-incremental/1391/
>> >
>> > Previous build was r222933, and was clean:
>> >
>> http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA-incremental/1390/
>> >
>> > (r222934 was formatting-only.)
>> >
>> > Our non-incremental builder started failing around then as well:
>> > http://lab.llvm.org:8080/green/job/clang-stage1-configure-RA/1251/
>> >
>> > Here's the failure:
>> >
>> > Failing Tests (34):
>> > LLVM :: CodeGen/X86/compact-unwind.ll
>> > LLVM :: CodeGen/X86/no-compact-unwind.ll
>> > LLVM :: ExecutionEngine/RuntimeDyld/X86/MachO_i386_eh_frame.s
>> > LLVM :: MC/ARM/basic-arm-instructions.s
>> > LLVM :: MC/ARM/basic-thumb-instructions.s
>> > LLVM :: MC/ARM/thumb-diagnostics.s
>> > LLVM :: MC/ARM/v8_IT_manual.s
>> > LLVM :: MC/COFF/seh-align3.s
>> > LLVM :: MC/COFF/seh-section.s
>> > LLVM :: MC/COFF/seh.s
>> > LLVM :: MC/ELF/relax-all-flag.s
>> > LLVM :: MC/ELF/relax.s
>> > LLVM :: MC/ELF/subsection.s
>> > LLVM :: MC/MachO/ARM/llvm-objdump-macho.s
>> > LLVM :: MC/MachO/ARM/long-call-branch-island-relocation.s
>> > LLVM :: MC/MachO/darwin-x86_64-diff-relocs.s
>> > LLVM :: MC/MachO/darwin-x86_64-nobase-relocs.s
>> > LLVM :: MC/MachO/direction_labels.s
>> > LLVM :: MC/MachO/jcc.s
>> > LLVM :: MC/MachO/relax-recompute-align.s
>> > LLVM :: MC/MachO/reloc.s
>> > LLVM :: MC/Mips/micromips-16-bit-instructions.s
>> > LLVM :: MC/Mips/micromips-control-instructions.s
>> > LLVM :: MC/X86/AlignedBundling/long-nop-pad.s
>> > LLVM :: MC/X86/AlignedBundling/relax-at-bundle-end.s
>> > LLVM :: MC/X86/AlignedBundling/relax-in-bundle-group.s
>> > LLVM :: MC/X86/AlignedBundling/single-inst-bundling.s
>> > LLVM :: MC/X86/avx512-encodings.s
>> > LLVM :: MC/X86/intel-syntax-encoding.s
>> > LLVM :: MC/X86/stackmap-nops.ll
>> > LLVM :: MC/X86/x86-64-avx512bw.s
>> > LLVM :: MC/X86/x86-64-avx512bw_vl.s
>> > LLVM :: MC/X86/x86-64-avx512f_vl.s
>> > LLVM :: MC/X86/x86-64.s
>> >
>> > Let me know if you need help reproducing.
>> >
>> >> On 2014 Nov 28, at 13:20, 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
>> >
>>
>> _______________________________________________
> 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/20141203/6a5c7086/attachment.html>
More information about the llvm-commits
mailing list