[llvm] 3f919ff - Revert "[TableGen] Add support for the 'assert' statement in multiclasses"
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 13 23:44:36 PDT 2021
Please include some details in the commit message describing why the
patch was reverted (helpful for folks following along in case they're
seeing a local failure that might be addressed by this patch - or
future authors looking to do something similar can see what the
pitfalls were)
On Thu, Apr 8, 2021 at 10:59 AM Paul C. Anagnostopoulos via
llvm-commits <llvm-commits at lists.llvm.org> wrote:
>
>
> Author: Paul C. Anagnostopoulos
> Date: 2021-04-08T13:58:58-04:00
> New Revision: 3f919ff25013d417ea050019154d28b2d7492f6b
>
> URL: https://github.com/llvm/llvm-project/commit/3f919ff25013d417ea050019154d28b2d7492f6b
> DIFF: https://github.com/llvm/llvm-project/commit/3f919ff25013d417ea050019154d28b2d7492f6b.diff
>
> LOG: Revert "[TableGen] Add support for the 'assert' statement in multiclasses"
>
> This reverts commit 3b9a15d910a8c748b1444333a4a3905a996528bc.
>
> Added:
>
>
> Modified:
> llvm/docs/TableGen/ProgRef.rst
> llvm/include/llvm/TableGen/Record.h
> llvm/lib/TableGen/Record.cpp
> llvm/lib/TableGen/TGParser.cpp
> llvm/lib/TableGen/TGParser.h
> llvm/test/TableGen/assert.td
>
> Removed:
>
>
>
> ################################################################################
> diff --git a/llvm/docs/TableGen/ProgRef.rst b/llvm/docs/TableGen/ProgRef.rst
> index c9bee3fa31bb0..c316c33a37f46 100644
> --- a/llvm/docs/TableGen/ProgRef.rst
> +++ b/llvm/docs/TableGen/ProgRef.rst
> @@ -1282,9 +1282,7 @@ placement.
> the subclasses and records that inherit from the class. The assertions are
> then checked when the records are completely built.
>
> -* In a multiclass definition, the assertions are saved with the other
> - components of the multiclass and then checked each time the multiclass
> - is instantiated with ``defm``.
> +* In a multiclass definition, ... [this placement is not yet available]
>
> Using assertions in TableGen files can simplify record checking in TableGen
> backends. Here is an example of an ``assert`` in two class definitions.
>
> diff --git a/llvm/include/llvm/TableGen/Record.h b/llvm/include/llvm/TableGen/Record.h
> index 36c6c688cf6e7..9432de6738613 100644
> --- a/llvm/include/llvm/TableGen/Record.h
> +++ b/llvm/include/llvm/TableGen/Record.h
> @@ -1469,10 +1469,6 @@ inline raw_ostream &operator<<(raw_ostream &OS, const RecordVal &RV) {
> }
>
> class Record {
> -public:
> - using AssertionTuple = std::tuple<SMLoc, Init *, Init *>;
> -
> -private:
> static unsigned LastID;
>
> Init *Name;
> @@ -1482,7 +1478,7 @@ class Record {
> SmallVector<Init *, 0> TemplateArgs;
> SmallVector<RecordVal, 0> Values;
> // Vector of [source location, condition Init, message Init].
> - SmallVector<AssertionTuple, 0> Assertions;
> + SmallVector<std::tuple<SMLoc, Init *, Init *>, 0> Assertions;
>
> // All superclasses in the inheritance forest in post-order (yes, it
> // must be a forest; diamond-shaped inheritance is not allowed).
> @@ -1557,7 +1553,7 @@ class Record {
>
> ArrayRef<RecordVal> getValues() const { return Values; }
>
> - ArrayRef<AssertionTuple> getAssertions() const {
> + ArrayRef<std::tuple<SMLoc, Init *, Init *>> getAssertions() const {
> return Assertions;
> }
>
> @@ -1624,7 +1620,7 @@ class Record {
> Assertions.append(Rec->Assertions);
> }
>
> - void checkRecordAssertions();
> + void checkAssertions();
>
> bool isSubClassOf(const Record *R) const {
> for (const auto &SCPair : SuperClasses)
>
> diff --git a/llvm/lib/TableGen/Record.cpp b/llvm/lib/TableGen/Record.cpp
> index 2c8234da84311..21f7bacb10b0f 100644
> --- a/llvm/lib/TableGen/Record.cpp
> +++ b/llvm/lib/TableGen/Record.cpp
> @@ -1835,7 +1835,7 @@ DefInit *VarDefInit::instantiate() {
> Records.addDef(std::move(NewRecOwner));
>
> // Check the assertions.
> - NewRec->checkRecordAssertions();
> + NewRec->checkAssertions();
>
> Def = DefInit::get(NewRec);
> }
> @@ -2618,7 +2618,7 @@ DagInit *Record::getValueAsDag(StringRef FieldName) const {
> // and message, then call CheckAssert().
> // Note: The condition and message are probably already resolved,
> // but resolving again allows calls before records are resolved.
> -void Record::checkRecordAssertions() {
> +void Record::checkAssertions() {
> RecordResolver R(*this);
> R.setFinal(true);
>
>
> diff --git a/llvm/lib/TableGen/TGParser.cpp b/llvm/lib/TableGen/TGParser.cpp
> index 8c2ec289ea854..3d95ac30ebd0b 100644
> --- a/llvm/lib/TableGen/TGParser.cpp
> +++ b/llvm/lib/TableGen/TGParser.cpp
> @@ -339,38 +339,27 @@ bool TGParser::AddSubMultiClass(MultiClass *CurMC,
> return resolve(SMC->Entries, TemplateArgs, false, &CurMC->Entries);
> }
>
> -/// Add a record, foreach loop, or assertion to the current context.
> +/// Add a record or foreach loop to the current context (global record keeper,
> +/// current inner-most foreach loop, or multiclass).
> bool TGParser::addEntry(RecordsEntry E) {
> - assert((!!E.Rec + !!E.Loop + !!E.Assertion) == 1 &&
> - "RecordsEntry has invalid number of items");
> + assert(!E.Rec || !E.Loop);
>
> - // If we are parsing a loop, add it to the loop's entries.
> if (!Loops.empty()) {
> Loops.back()->Entries.push_back(std::move(E));
> return false;
> }
>
> - // If it is a loop, then resolve and perform the loop.
> if (E.Loop) {
> SubstStack Stack;
> return resolve(*E.Loop, Stack, CurMultiClass == nullptr,
> CurMultiClass ? &CurMultiClass->Entries : nullptr);
> }
>
> - // If we are parsing a multiclass, add it to the multiclass's entries.
> if (CurMultiClass) {
> CurMultiClass->Entries.push_back(std::move(E));
> return false;
> }
>
> - // If it is an assertion, then it's a top-level one, so check it.
> - if (E.Assertion) {
> - CheckAssert(std::get<0>(*E.Assertion), std::get<1>(*E.Assertion),
> - std::get<2>(*E.Assertion));
> - return false;
> - }
> -
> - // It must be a record, so finish it off.
> return addDefOne(std::move(E.Rec));
> }
>
> @@ -425,24 +414,6 @@ bool TGParser::resolve(const std::vector<RecordsEntry> &Source,
> for (auto &E : Source) {
> if (E.Loop) {
> Error = resolve(*E.Loop, Substs, Final, Dest);
> -
> - } else if (E.Assertion) {
> - MapResolver R;
> - for (const auto &S : Substs)
> - R.set(S.first, S.second);
> - Init *Condition = std::get<1>(*E.Assertion)->resolveReferences(R);
> - Init *Message = std::get<2>(*E.Assertion)->resolveReferences(R);
> -
> - if (Dest) {
> - std::unique_ptr<Record::AssertionTuple> tuple =
> - std::make_unique<Record::AssertionTuple>(std::get<0>(*E.Assertion),
> - std::move(Condition),
> - std::move(Message));
> - Dest->push_back(std::move(tuple));
> - } else {
> - CheckAssert(std::get<0>(*E.Assertion), Condition, Message);
> - }
> -
> } else {
> auto Rec = std::make_unique<Record>(*E.Rec);
> if (Loc)
> @@ -488,7 +459,7 @@ bool TGParser::addDefOne(std::unique_ptr<Record> Rec) {
> }
>
> // Check the assertions.
> - Rec->checkRecordAssertions();
> + Rec->checkAssertions();
>
> // If ObjectBody has template arguments, it's an error.
> assert(Rec->getTemplateArgs().empty() && "How'd this get template args?");
> @@ -2871,15 +2842,10 @@ bool TGParser::ApplyLetStack(Record *CurRec) {
> return false;
> }
>
> -/// Apply the current let bindings to the RecordsEntry.
> bool TGParser::ApplyLetStack(RecordsEntry &Entry) {
> if (Entry.Rec)
> return ApplyLetStack(Entry.Rec.get());
>
> - // Let bindings are not applied to assertions.
> - if (Entry.Assertion)
> - return false;
> -
> for (auto &E : Entry.Loop->Entries) {
> if (ApplyLetStack(E))
> return true;
> @@ -2923,8 +2889,8 @@ bool TGParser::ParseObjectBody(Record *CurRec) {
> return ParseBody(CurRec);
> }
>
> -/// ParseDef - Parse and return a top level or multiclass record definition.
> -/// Return false if okay, true if error.
> +/// ParseDef - Parse and return a top level or multiclass def, return the record
> +/// corresponding to it. This returns null on error.
> ///
> /// DefInst ::= DEF ObjectName ObjectBody
> ///
> @@ -3218,12 +3184,12 @@ bool TGParser::ParseAssert(MultiClass *CurMultiClass, Record *CurRec) {
> if (!consume(tgtok::semi))
> return TokError("expected ';'");
>
> - if (CurRec) {
> + if (CurMultiClass) {
> + assert(false && "assert in multiclass not yet supported");
> + } else if (CurRec) {
> CurRec->addAssertion(ConditionLoc, Condition, Message);
> - } else {
> - std::unique_ptr<Record::AssertionTuple> tuple =
> - std::make_unique<Record::AssertionTuple>(ConditionLoc, Condition, Message);
> - addEntry(std::move(tuple));
> + } else { // at top level
> + CheckAssert(ConditionLoc, Condition, Message);
> }
>
> return false;
> @@ -3362,13 +3328,10 @@ bool TGParser::ParseTopLevelLet(MultiClass *CurMultiClass) {
> /// MultiClassInst ::= MULTICLASS ID TemplateArgList?
> /// ':' BaseMultiClassList '{' MultiClassObject+ '}'
> /// MultiClassObject ::= DefInst
> +/// MultiClassObject ::= MultiClassInst
> /// MultiClassObject ::= DefMInst
> -/// MultiClassObject ::= Defvar
> -/// MultiClassObject ::= Foreach
> -/// MultiClassObject ::= If
> /// MultiClassObject ::= LETCommand '{' ObjectList '}'
> /// MultiClassObject ::= LETCommand Object
> -/// MultiClassObject ::= Assert
> ///
> bool TGParser::ParseMultiClass() {
> assert(Lex.getCode() == tgtok::MultiClass && "Unexpected token");
> @@ -3433,6 +3396,8 @@ bool TGParser::ParseMultiClass() {
> default:
> return TokError("expected 'assert', 'def', 'defm', 'defvar', "
> "'foreach', 'if', or 'let' in multiclass body");
> + case tgtok::Assert:
> + return TokError("an assert statement in a multiclass is not yet supported");
>
> case tgtok::Def:
> case tgtok::Defm:
> @@ -3440,7 +3405,6 @@ bool TGParser::ParseMultiClass() {
> case tgtok::Foreach:
> case tgtok::If:
> case tgtok::Let:
> - case tgtok::Assert:
> if (ParseObject(CurMultiClass))
> return true;
> break;
> @@ -3600,7 +3564,7 @@ bool TGParser::ParseObject(MultiClass *MC) {
> default:
> return TokError(
> "Expected assert, class, def, defm, defset, foreach, if, or let");
> - case tgtok::Assert: return ParseAssert(MC);
> + case tgtok::Assert: return ParseAssert(MC, nullptr);
> case tgtok::Def: return ParseDef(MC);
> case tgtok::Defm: return ParseDefm(MC);
> case tgtok::Defvar: return ParseDefvar();
>
> diff --git a/llvm/lib/TableGen/TGParser.h b/llvm/lib/TableGen/TGParser.h
> index 254ce3c90980e..6514d4276a7f6 100644
> --- a/llvm/lib/TableGen/TGParser.h
> +++ b/llvm/lib/TableGen/TGParser.h
> @@ -36,12 +36,10 @@ namespace llvm {
> }
> };
>
> - /// RecordsEntry - Holds exactly one of a Record, ForeachLoop, or
> - /// assertion tuple.
> + /// RecordsEntry - Can be either a record or a foreach loop.
> struct RecordsEntry {
> std::unique_ptr<Record> Rec;
> std::unique_ptr<ForeachLoop> Loop;
> - std::unique_ptr<Record::AssertionTuple> Assertion;
>
> void dump() const;
>
> @@ -49,8 +47,6 @@ namespace llvm {
> RecordsEntry(std::unique_ptr<Record> Rec) : Rec(std::move(Rec)) {}
> RecordsEntry(std::unique_ptr<ForeachLoop> Loop)
> : Loop(std::move(Loop)) {}
> - RecordsEntry(std::unique_ptr<Record::AssertionTuple> Assertion)
> - : Assertion(std::move(Assertion)) {}
> };
>
> /// ForeachLoop - Record the iteration state associated with a for loop.
> @@ -226,7 +222,7 @@ class TGParser {
> bool ParseForeach(MultiClass *CurMultiClass);
> bool ParseIf(MultiClass *CurMultiClass);
> bool ParseIfBody(MultiClass *CurMultiClass, StringRef Kind);
> - bool ParseAssert(MultiClass *CurMultiClass, Record *CurRec = nullptr);
> + bool ParseAssert(MultiClass *CurMultiClass, Record *CurRec);
> bool ParseTopLevelLet(MultiClass *CurMultiClass);
> void ParseLetList(SmallVectorImpl<LetRecord> &Result);
>
>
> diff --git a/llvm/test/TableGen/assert.td b/llvm/test/TableGen/assert.td
> index fb8f7beccc89c..1c436301d85d0 100644
> --- a/llvm/test/TableGen/assert.td
> +++ b/llvm/test/TableGen/assert.td
> @@ -39,15 +39,6 @@ class Cube<int n> {
>
> assert !eq(Cube<9>.result, 81), "cube of 9 should be 729";
>
> -// CHECK: assertion failed
> -// CHECK: note: foreach i cannot be 2
> -// CHECK-NOT: note: foreach i cannot be 2
> -
> -foreach i = 1...3 in {
> - assert !ne(i, 2), "foreach i cannot be 2";
> - def bar_ # i;
> -}
> -
> // Test the assert statement in a record definition.
>
> // CHECK: assertion failed
> @@ -145,47 +136,3 @@ def Rec32 {
>
> // Test the assert statement in a multiclass.
>
> -// CHECK: assertion failed
> -// CHECK: note: MC1 id string is too long
> -// CHECK: assertion failed
> -// CHECK: note: MC1 seq is too high
> -
> -multiclass MC1<string id, int seq> {
> - assert !le(!size(id), 5), "MC1 id string is too long";
> - assert !le(seq, 999999), "MC1 seq is too high";
> -
> - def _mc1 {
> - string ID = id;
> - int Seq = seq;
> - }
> -}
> -
> -defm Rec40 : MC1<"ILISP", 999>;
> -defm Rec41 : MC1<"ILISPX", 999>;
> -defm Rec42 : MC1<"ILISP", 999999999>;
> -
> -// CHECK: assertion failed
> -// CHECK: note: MC2 phrase must be secret: secrex code
> -
> -multiclass MC2<string phr> {
> - assert !eq(!substr(phr, 0, 6), "secret"), "MC2 phrase must be secret: " # phr;
> -
> - def _mc2 {
> - string phrase = phr;
> - }
> -}
> -
> -multiclass MC3<string phr> {
> - defm _mc3 : MC2<phr>;
> -}
> -
> -defm Rec43 : MC3<"secrex code">;
> -
> -// CHECK: assertion failed
> -// CHECK: note: MC2 phrase must be secret: xecret code
> -
> -multiclass MC4<string phr> : MC2<phr> {
> - def _def;
> -}
> -
> -defm Rec44 : MC4<"xecret code">;
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list