From 5f0e198211584dc3305789957b8fb2f6a3b3a4bc Mon Sep 17 00:00:00 2001 From: Rahul Joshi Date: Thu, 3 Oct 2024 10:20:43 -0700 Subject: [PATCH] [TableGen] Print record location when record asserts fail When record assertions fail, print an error message with the record's location, so its easier to see where the record that caused the assert to fail was instantiated. This is useful when the assert condition in a class depends on a template parameter, so we need to know the context of the definition to determine why the assert failed. Also enhanced the assert.td test to check for these context messages, and also add checks for some assert failures that were missing in the test. --- llvm/include/llvm/TableGen/Error.h | 3 ++- llvm/lib/TableGen/Error.cpp | 10 ++++--- llvm/lib/TableGen/Record.cpp | 10 ++++++- llvm/test/TableGen/assert.td | 43 +++++++++++++++++++----------- 4 files changed, 46 insertions(+), 20 deletions(-) diff --git a/llvm/include/llvm/TableGen/Error.h b/llvm/include/llvm/TableGen/Error.h index a51ed48dba0e8..512249b0160c2 100644 --- a/llvm/include/llvm/TableGen/Error.h +++ b/llvm/include/llvm/TableGen/Error.h @@ -48,7 +48,8 @@ void PrintError(const RecordVal *RecVal, const Twine &Msg); [[noreturn]] void PrintFatalError(const RecordVal *RecVal, const Twine &Msg); [[noreturn]] void PrintFatalError(function_ref PrintMsg); -void CheckAssert(SMLoc Loc, Init *Condition, Init *Message); +// Returns true if the assert failed. +bool CheckAssert(SMLoc Loc, Init *Condition, Init *Message); void dumpMessage(SMLoc Loc, Init *Message); extern SourceMgr SrcMgr; diff --git a/llvm/lib/TableGen/Error.cpp b/llvm/lib/TableGen/Error.cpp index e21f369833be6..45cdcc8ae2619 100644 --- a/llvm/lib/TableGen/Error.cpp +++ b/llvm/lib/TableGen/Error.cpp @@ -160,18 +160,22 @@ void PrintFatalError(const RecordVal *RecVal, const Twine &Msg) { // Check an assertion: Obtain the condition value and be sure it is true. // If not, print a nonfatal error along with the message. -void CheckAssert(SMLoc Loc, Init *Condition, Init *Message) { +bool CheckAssert(SMLoc Loc, Init *Condition, Init *Message) { auto *CondValue = dyn_cast_or_null(Condition->convertInitializerTo( IntRecTy::get(Condition->getRecordKeeper()))); - if (!CondValue) + if (!CondValue) { PrintError(Loc, "assert condition must of type bit, bits, or int."); - else if (!CondValue->getValue()) { + return true; + } + if (!CondValue->getValue()) { PrintError(Loc, "assertion failed"); if (auto *MessageInit = dyn_cast(Message)) PrintNote(MessageInit->getValue()); else PrintNote("(assert message is not a string)"); + return true; } + return false; } // Dump a message to stderr. diff --git a/llvm/lib/TableGen/Record.cpp b/llvm/lib/TableGen/Record.cpp index bebfb4b8c381c..9307987df6a87 100644 --- a/llvm/lib/TableGen/Record.cpp +++ b/llvm/lib/TableGen/Record.cpp @@ -3182,11 +3182,19 @@ void Record::checkRecordAssertions() { RecordResolver R(*this); R.setFinal(true); + bool AnyFailed = false; for (const auto &Assertion : getAssertions()) { Init *Condition = Assertion.Condition->resolveReferences(R); Init *Message = Assertion.Message->resolveReferences(R); - CheckAssert(Assertion.Loc, Condition, Message); + AnyFailed |= CheckAssert(Assertion.Loc, Condition, Message); } + + if (!AnyFailed) + return; + + // If any of the record assertions failed, print some context that will + // help see where the record that caused these assert failures is defined. + PrintError(this, "assertion failed in this record"); } void Record::emitRecordDumps() { diff --git a/llvm/test/TableGen/assert.td b/llvm/test/TableGen/assert.td index d616adcb5d0d1..17c08b4313531 100644 --- a/llvm/test/TableGen/assert.td +++ b/llvm/test/TableGen/assert.td @@ -1,6 +1,8 @@ -// RUN: not llvm-tblgen %s 2>&1 | FileCheck %s +// RUN: not llvm-tblgen %s 2>&1 | FileCheck %s -DFILE=%s +// ----------------------------------------------------------------------------- // Test the assert statement at top level. +// ----------------------------------------------------------------------------- // CHECK: assertion failed // CHECK-NOT: note: primary name is too short @@ -48,14 +50,16 @@ foreach i = 1...3 in { def bar_ # i; } +// ----------------------------------------------------------------------------- // Test the assert statement in a record definition. +// ----------------------------------------------------------------------------- -// CHECK: assertion failed +// CHECK: [[FILE]]:[[@LINE+8]]:10: error: assertion failed // CHECK-NOT: primary first name is not "Grace" -// CHECK: primary first name is not "Grack" -// CHECK: assertion failed -// CHECK: foo field should be - +// CHECK: note: primary first name is not "Grack" +// CHECK: [[FILE]]:[[@LINE+7]]:10: error: assertion failed +// CHECK: note: foo field should be +// CHECK: [[FILE]]:[[@LINE+1]]:5: error: assertion failed in this record def Rec10 { assert !eq(!substr(Name, 0, 5), "Grace"), "primary first name is not \"Grace\""; assert !eq(!substr(Name, 0, 5), "Grack"), "primary first name is not \"Grack\""; @@ -63,18 +67,18 @@ def Rec10 { assert !eq(foo, "foo"), "foo field should be \"Foo\""; } -// CHECK: assertion failed +// CHECK: [[FILE]]:[[@LINE+5]]:10: error: assertion failed // CHECK: note: magic field is incorrect: 42 - +// CHECK: [[FILE]]:[[@LINE+1]]:5: error: assertion failed in this record def Rec11 { int magic = 13; assert !eq(magic, 13), "magic field is incorrect: " # magic; let magic = 42; } -// CHECK: assertion failed +// CHECK: [[FILE]]:[[@LINE+6]]:10: error: assertion failed // CHECK: note: var field has wrong value - +// CHECK: [[FILE]]:[[@LINE+1]]:5: error: assertion failed in this record def Rec12 { defvar prefix = "foo_"; string var = prefix # "snork"; @@ -83,11 +87,11 @@ def Rec12 { // CHECK: assertion failed // CHECK: note: kind field has wrong value - class Kind { int kind = 7; } +// CHECK: [[FILE]]:[[@LINE+1]]:5: error: assertion failed in this record def Rec13 : Kind { let kind = 8; assert !eq(kind, 7), "kind field has wrong value: " # kind; @@ -95,13 +99,15 @@ def Rec13 : Kind { // CHECK: assertion failed // CHECK: note: double_result should be - +// CHECK: [[FILE]]:[[@LINE+1]]:5: error: assertion failed in this record def Rec14 : Cube<3> { int double_result = !mul(result, 2); assert !eq(double_result, 53), "double_result should be 54"; } +// ----------------------------------------------------------------------------- // Test the assert statement in a class definition. +// ----------------------------------------------------------------------------- class PersonName { assert !le(!size(name), 32), "person name is too long: " # name; @@ -118,32 +124,39 @@ def Rec20 : Person<"Donald Knuth", 60>; // CHECK: assertion failed // CHECK: note: person name is too long - +// CHECK: [[FILE]]:[[@LINE+1]]:5: error: assertion failed in this record def Rec21 : Person<"Donald Uh Oh This Name Is Too Long Knuth", 50>; // CHECK: assertion failed // CHECK: note: person age is invalid - +// CHECK: [[FILE]]:[[@LINE+1]]:5: error: assertion failed in this record def Rec22 : Person<"Donald Knuth", 150>; // Test the assert statement in an anonymous class invocation. - def Rec30 { string Name = Person<"Margaret Heafield Hamilton", 25>.Name; int Age = Person<"Margaret Heafield Hamilton", 25>.Age; } +// CHECK: assertion failed +// CHECK: note: person name is too long +// CHECK: [[FILE]]:[[@LINE+2]]:17: error: assertion failed in this record def Rec31 { string Name = Person<"Margaret Heafield And More Middle Names Hamilton", 25>.Name; int Age = Person<"Margaret Heafield Hamilton", 25>.Age; } +// CHECK: assertion failed +// CHECK: note: person age is invalid: 0 +// CHECK: [[FILE]]:[[@LINE+3]]:13: error: assertion failed in this record def Rec32 { string Name = Person<"Margaret Heafield Hamilton", 25>.Name; int Age = Person<"Margaret Heafield Hamilton", 0>.Age; } +// ----------------------------------------------------------------------------- // Test the assert statement in a multiclass. +// ----------------------------------------------------------------------------- // CHECK: assertion failed // CHECK: note: MC1 id string is too long