Skip to content

Commit c72c998

Browse files
nhaehnlenhaehnle
committed
tablegen: optimize the happy path in verifying logical OR expressions
Instead of unconditionally collecting the error messages for all branches of a LogicOr and then throwing them all away if one of them was successful, we generate the code for checking the branches twice. For the initial check, no error messages are printed. Then, only if all branches failed, the checks are run again, printing error messages this time.
1 parent 0a4db2f commit c72c998

File tree

5 files changed

+205
-115
lines changed

5 files changed

+205
-115
lines changed

‎include/llvm-dialects/TableGen/Evaluator.h‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -127,17 +127,17 @@ class Evaluator {
127127
std::string evaluate(Variable *variable);
128128

129129
/// Emit C++ code that checks all known constraints. Errors are written to
130-
/// $_errs, and false is returned by the generated code if an error is
131-
/// detected.
132-
bool check();
130+
/// $_errs if writeErrs is true, and false is returned by the generated code
131+
/// if an error is detected.
132+
bool check(bool writeErrs);
133133

134134
private:
135135
void checkErrors();
136136
std::string evaluateImpl(Variable *variable);
137-
bool checkApplyEvaluate(const Apply *apply);
138-
bool checkApplyCapture(const Apply *apply);
139-
bool checkLogicOr(const LogicOr *logicOr);
140-
void checkAssignment(Variable *variable, std::string value,
137+
bool checkApplyEvaluate(bool writeErrs, const Apply *apply);
138+
bool checkApplyCapture(bool writeErrs, const Apply *apply);
139+
bool checkLogicOr(bool writeErrs, const LogicOr *logicOr);
140+
void checkAssignment(bool writeErrs, Variable *variable, std::string value,
141141
llvm::Init *constraint);
142142
};
143143

‎lib/TableGen/DialectType.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ void DialectType::emitDefinition(raw_ostream &out, GenDialect *dialect) const {
312312
assignment.assign(variable, fmt.getSubstFor("name").value());
313313
}
314314

315-
eval.check();
315+
eval.check(true);
316316

317317
out << "return true;\n}\n\n";
318318
}

‎lib/TableGen/Evaluator.cpp‎

Lines changed: 103 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ std::string Evaluator::evaluateImpl(Variable *goal) {
305305
return tgfmt(cpp->getEvaluate(), &m_fmt).str();
306306
}
307307

308-
bool Evaluator::check() {
308+
bool Evaluator::check(bool writeErrs) {
309309
if (m_comments) {
310310
m_out << "// Checking the constraint system:\n";
311311
m_system.print(m_out, "// ");
@@ -340,7 +340,7 @@ bool Evaluator::check() {
340340
m_out << "// Try apply-evaluate of " << apply->toString() << '\n';
341341
}
342342

343-
if (checkApplyEvaluate(apply)) {
343+
if (checkApplyEvaluate(writeErrs, apply)) {
344344
if (m_comments) {
345345
m_out << "// Done: " << apply->toString() << '\n';
346346
}
@@ -363,10 +363,10 @@ bool Evaluator::check() {
363363

364364
bool ok = false;
365365
if (auto *apply = dyn_cast<Apply>(constraint)) {
366-
ok = checkApplyCapture(apply);
366+
ok = checkApplyCapture(writeErrs, apply);
367367
} else {
368368
auto *logicOr = dyn_cast<LogicOr>(constraint);
369-
ok = checkLogicOr(logicOr);
369+
ok = checkLogicOr(writeErrs, logicOr);
370370
}
371371

372372
if (ok) {
@@ -391,7 +391,7 @@ bool Evaluator::check() {
391391
}
392392

393393
/// Check apply constraints by evaluating them where possible.
394-
bool Evaluator::checkApplyEvaluate(const Apply *apply) {
394+
bool Evaluator::checkApplyEvaluate(bool writeErrs, const Apply *apply) {
395395
if (!apply->getPredicate()->canDerive(0))
396396
return false;
397397

@@ -424,7 +424,7 @@ bool Evaluator::checkApplyEvaluate(const Apply *apply) {
424424
nestedAssignment.assign(tg->variables()[argIdx],
425425
m_assignment.lookup(apply->arguments()[argIdx]));
426426
}
427-
nestedEvaluator.check();
427+
nestedEvaluator.check(writeErrs);
428428

429429
value = nestedAssignment.lookup(tg->variables()[0]).str();
430430
} else {
@@ -439,13 +439,13 @@ bool Evaluator::checkApplyEvaluate(const Apply *apply) {
439439
}
440440

441441
Variable *self = apply->arguments()[0];
442-
checkAssignment(self, std::move(value), apply->getInit());
442+
checkAssignment(writeErrs, self, std::move(value), apply->getInit());
443443
return true;
444444
}
445445

446446
/// Check apply constraints by checking and capturing them from the self
447447
/// argument if possible.
448-
bool Evaluator::checkApplyCapture(const Apply *apply) {
448+
bool Evaluator::checkApplyCapture(bool writeErrs, const Apply *apply) {
449449
Variable *self = apply->arguments()[0];
450450
StringRef selfValue = m_assignment.lookup(self);
451451
if (selfValue.empty()) {
@@ -466,19 +466,21 @@ bool Evaluator::checkApplyCapture(const Apply *apply) {
466466
}
467467

468468
std::string with;
469-
raw_string_ostream withstream(with);
470-
withstream << tgfmt(R"(
471-
$_errs << " with $0 = " << printable($1) << '\n';
472-
)",
473-
&m_fmt, self->toString(), selfValue);
474-
if (!apply->getPredicate()->canCheckFromSelf()) {
475-
for (unsigned argIdx = 1; argIdx < apply->arguments().size(); ++argIdx) {
476-
Variable *argument = apply->arguments()[argIdx];
477-
withstream << tgfmt(R"(
478-
$_errs << " with $0 = " << printable($1) << '\n';
479-
)",
480-
&m_fmt, argument->toString(),
481-
m_assignment.lookup(argument));
469+
if (writeErrs) {
470+
raw_string_ostream withstream(with);
471+
withstream << tgfmt(R"(
472+
$_errs << " with $0 = " << printable($1) << '\n';
473+
)",
474+
&m_fmt, self->toString(), selfValue);
475+
if (!apply->getPredicate()->canCheckFromSelf()) {
476+
for (unsigned argIdx = 1; argIdx < apply->arguments().size(); ++argIdx) {
477+
Variable *argument = apply->arguments()[argIdx];
478+
withstream << tgfmt(R"(
479+
$_errs << " with $0 = " << printable($1) << '\n';
480+
)",
481+
&m_fmt, argument->toString(),
482+
m_assignment.lookup(argument));
483+
}
482484
}
483485
}
484486

@@ -499,7 +501,7 @@ bool Evaluator::checkApplyCapture(const Apply *apply) {
499501

500502
m_out << "if (![&]() {\n";
501503

502-
nestedEvaluator.check();
504+
nestedEvaluator.check(writeErrs);
503505

504506
{
505507
FmtContextScope scope{m_fmt};
@@ -508,21 +510,29 @@ bool Evaluator::checkApplyCapture(const Apply *apply) {
508510

509511
m_out << tgfmt(R"(
510512
return true;
511-
}()) {
513+
}()) {)",
514+
&m_fmt);
515+
516+
if (writeErrs) {
517+
m_out << tgfmt(R"(
512518
$_errs << " while checking $constraint\n";
513519
$with
520+
)",
521+
&m_fmt);
522+
}
523+
524+
m_out << R"(
514525
return false;
515526
}
516-
)",
517-
&m_fmt);
527+
)";
518528
}
519529

520530
for (unsigned argIdx = 1; argIdx < apply->arguments().size(); ++argIdx) {
521531
if (!tg->canDerive(argIdx))
522532
continue;
523533

524534
StringRef value = nestedAssignment.lookup(tg->variables()[argIdx]);
525-
checkAssignment(apply->arguments()[argIdx], value.str(),
535+
checkAssignment(writeErrs, apply->arguments()[argIdx], value.str(),
526536
apply->getInit());
527537
}
528538
} else {
@@ -548,13 +558,21 @@ bool Evaluator::checkApplyCapture(const Apply *apply) {
548558
m_fmt.addSubst("with", with);
549559

550560
m_out << tgfmt(R"(
551-
if (!($checkValue)) {
561+
if (!($checkValue)) {)",
562+
&m_fmt);
563+
564+
if (writeErrs) {
565+
m_out << tgfmt(R"(
552566
$_errs << " failed check for $constraint\n";
553567
$with
568+
)",
569+
&m_fmt);
570+
}
571+
572+
m_out << R"(
554573
return false;
555574
}
556-
)",
557-
&m_fmt);
575+
)";
558576
}
559577

560578
for (unsigned argIdx = 1; argIdx < apply->arguments().size(); ++argIdx) {
@@ -563,14 +581,14 @@ bool Evaluator::checkApplyCapture(const Apply *apply) {
563581
continue;
564582

565583
std::string value = tgfmt(cpp->getCapture(argIdx), &m_fmt).str();
566-
checkAssignment(variable, std::move(value), apply->getInit());
584+
checkAssignment(writeErrs, variable, std::move(value), apply->getInit());
567585
}
568586
}
569587

570588
return true;
571589
}
572590

573-
bool Evaluator::checkLogicOr(const LogicOr *logicOr) {
591+
bool Evaluator::checkLogicOr(bool writeErrs, const LogicOr *logicOr) {
574592
for (Variable *variable : logicOr->variables()) {
575593
if (m_assignment.lookup(variable).empty()) {
576594
m_errs << "cannot check " << logicOr->toString() << '\n';
@@ -580,12 +598,9 @@ bool Evaluator::checkLogicOr(const LogicOr *logicOr) {
580598
}
581599

582600
SymbolScope symbolScope(&m_symbols);
583-
FmtContextScope scope{m_fmt};
584-
m_fmt.addSubst("_orErrors", m_symbols->chooseName("orErrors"));
585601
{
586602
m_out << tgfmt(R"(
587603
{
588-
std::array<std::string, $0> $_orErrors;
589604
if (true
590605
)",
591606
&m_fmt, logicOr->branches().size());
@@ -597,14 +612,13 @@ bool Evaluator::checkLogicOr(const LogicOr *logicOr) {
597612

598613
m_out << tgfmt(R"(
599614
&& !([&]() {
600-
::llvm::raw_string_ostream $_errs($_orErrors[$0]);
601615
)",
602616
&m_fmt, index);
603617

604618
Assignment scopedAssignment{&m_assignment};
605619
Evaluator childEvaluator(*m_symbols, scopedAssignment, childSystem, m_out,
606620
m_fmt);
607-
if (!childEvaluator.check()) {
621+
if (!childEvaluator.check(false)) {
608622
m_errs << "failed to check branch #" << index << " ("
609623
<< logicOr->branchInits()[index]->getAsString() << "):\n";
610624
m_errs << childEvaluator.takeErrorMessages();
@@ -616,32 +630,45 @@ bool Evaluator::checkLogicOr(const LogicOr *logicOr) {
616630

617631
m_out << ") {\n";
618632

619-
if (Variable *self = logicOr->getSelf()) {
620-
FmtContextScope scope{m_fmt};
621-
m_fmt.addSubst("name", self->toString());
622-
m_fmt.addSubst("value", m_assignment.lookup(self));
633+
if (writeErrs) {
634+
if (Variable *self = logicOr->getSelf()) {
635+
FmtContextScope scope{m_fmt};
636+
m_fmt.addSubst("name", self->toString());
637+
m_fmt.addSubst("value", m_assignment.lookup(self));
623638

624-
m_out << tgfmt(R"(
625-
$_errs << " $name (" << printable($value)
626-
<< ") does not match any available option\n";
627-
)",
628-
&m_fmt);
629-
}
639+
m_out << tgfmt(R"(
640+
$_errs << " $name (" << printable($value)
641+
<< ") does not match any available option\n";
642+
)",
643+
&m_fmt);
644+
}
630645

631-
for (unsigned i = 0; i < logicOr->branches().size(); ++i) {
632-
m_out << tgfmt(R"(
633-
$_errs << " failed option $0 ($1):\n"
634-
<< $_orErrors[$0];
635-
)",
636-
&m_fmt, i, logicOr->branchInits()[i]->getAsString());
646+
for (unsigned i = 0; i < logicOr->branches().size(); ++i) {
647+
m_out << tgfmt(R"(
648+
$_errs << " failed option $0 ($1):\n";
649+
([&]() {
650+
)",
651+
&m_fmt, i, logicOr->branchInits()[i]->getAsString());
652+
653+
const ConstraintSystem &childSystem = logicOr->branches()[i];
654+
Assignment scopedAssignment{&m_assignment};
655+
Evaluator childEvaluator(*m_symbols, scopedAssignment, childSystem, m_out,
656+
m_fmt);
657+
childEvaluator.check(true);
658+
659+
m_out << R"(
660+
return true;
661+
})();
662+
)";
663+
}
637664
}
638665

639666
m_out << "return false;\n}\n}\n";
640667
return true;
641668
}
642669

643-
void Evaluator::checkAssignment(Variable *variable, std::string value,
644-
Init *constraint) {
670+
void Evaluator::checkAssignment(bool writeErrs, Variable *variable,
671+
std::string value, Init *constraint) {
645672
assert(variable);
646673
assert(!value.empty());
647674

@@ -660,27 +687,43 @@ void Evaluator::checkAssignment(Variable *variable, std::string value,
660687

661688
if (variable->isNamed()) {
662689
m_out << tgfmt(R"(
663-
if ($new != $old) {
690+
if ($new != $old) {)",
691+
&m_fmt);
692+
693+
if (writeErrs) {
694+
m_out << tgfmt(R"(
664695
$_errs << " unexpected value of $variable:\n";
665696
$_errs << " expected: " << printable($new) << '\n';
666697
$_errs << " actual: " << printable($old) << '\n';
698+
)",
699+
&m_fmt);
700+
}
701+
702+
m_out << R"(
667703
return false;
668704
}
669-
)",
670-
&m_fmt);
705+
)";
671706
} else {
672707
m_fmt.addSubst("constraint", constraint->getAsString());
673708

674709
m_out << tgfmt(R"(
675-
if ($new != $old) {
710+
if ($new != $old) {)",
711+
&m_fmt);
712+
713+
if (writeErrs) {
714+
m_out << tgfmt(R"(
676715
$_errs << " inconsistent value of $variable found\n";
677716
$_errs << " while checking $constraint:\n";
678717
$_errs << " here: " << printable($new) << '\n';
679718
$_errs << " previously: " << printable($old) << '\n';
719+
)",
720+
&m_fmt);
721+
}
722+
723+
m_out << R"(
680724
return false;
681725
}
682-
)",
683-
&m_fmt);
726+
)";
684727
}
685728
}
686729
}

‎lib/TableGen/Operations.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -438,7 +438,7 @@ void Operation::emitVerifierMethod(llvm::raw_ostream &out,
438438
assignment.assign(systemVariable, name);
439439
}
440440

441-
if (!eval.check()) {
441+
if (!eval.check(true)) {
442442
llvm::errs() << "failed to generate verifier method for " << name << ":\n";
443443
llvm::errs() << eval.takeErrorMessages();
444444
report_fatal_error("failed to generate verifier");

0 commit comments

Comments
 (0)