From c60f112c9bbcf75da8eb8946339098a840769bd8 Mon Sep 17 00:00:00 2001 From: Nikhil Kalra Date: Mon, 24 Feb 2025 11:34:40 -0800 Subject: [PATCH 01/11] functional changes --- mlir/include/mlir/Bindings/Python/Diagnostics.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/mlir/include/mlir/Bindings/Python/Diagnostics.h b/mlir/include/mlir/Bindings/Python/Diagnostics.h index ea80e14dde0f3..47914e4107ebd 100644 --- a/mlir/include/mlir/Bindings/Python/Diagnostics.h +++ b/mlir/include/mlir/Bindings/Python/Diagnostics.h @@ -10,6 +10,7 @@ #define MLIR_BINDINGS_PYTHON_DIAGNOSTICS_H #include +#include #include #include "mlir-c/Diagnostics.h" @@ -45,6 +46,11 @@ class CollectDiagnosticsToStringScope { mlirLocationPrint(loc, printer, data); *static_cast(data) += ": "; mlirDiagnosticPrint(diag, printer, data); + for (intptr_t i = 0; i < mlirDiagnosticGetNumNotes(diag); i++) { + *static_cast(data) += "\n"; + MlirDiagnostic note = mlirDiagnosticGetNote(diag, i); + handler(note, data); + } return mlirLogicalResultSuccess(); } From 6d09f001f3fcc4ef0d5f1f799819ac78adbe16cf Mon Sep 17 00:00:00 2001 From: Nikhil Kalra Date: Mon, 24 Feb 2025 13:21:43 -0800 Subject: [PATCH 02/11] make more efficient --- .../mlir/Bindings/Python/Diagnostics.h | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/mlir/include/mlir/Bindings/Python/Diagnostics.h b/mlir/include/mlir/Bindings/Python/Diagnostics.h index 47914e4107ebd..4f9be844dc1ac 100644 --- a/mlir/include/mlir/Bindings/Python/Diagnostics.h +++ b/mlir/include/mlir/Bindings/Python/Diagnostics.h @@ -9,14 +9,14 @@ #ifndef MLIR_BINDINGS_PYTHON_DIAGNOSTICS_H #define MLIR_BINDINGS_PYTHON_DIAGNOSTICS_H +#include "mlir-c/Diagnostics.h" +#include "mlir-c/IR.h" + #include #include +#include #include -#include "mlir-c/Diagnostics.h" -#include "mlir-c/IR.h" -#include "llvm/ADT/StringRef.h" - namespace mlir { namespace python { @@ -29,25 +29,28 @@ class CollectDiagnosticsToStringScope { /*deleteUserData=*/nullptr); } ~CollectDiagnosticsToStringScope() { - assert(errorMessage.empty() && "unchecked error message"); mlirContextDetachDiagnosticHandler(context, handlerID); } - [[nodiscard]] std::string takeMessage() { return std::move(errorMessage); } + [[nodiscard]] std::string takeMessage() { + std::ostringstream stream; + std::swap(stream, errorMessage); + return stream.str(); + } private: static MlirLogicalResult handler(MlirDiagnostic diag, void *data) { auto printer = +[](MlirStringRef message, void *data) { - *static_cast(data) += - llvm::StringRef(message.data, message.length); + *static_cast(data) + << std::string_view(message.data, message.length); }; MlirLocation loc = mlirDiagnosticGetLocation(diag); - *static_cast(data) += "at "; + *static_cast(data) << "at "; mlirLocationPrint(loc, printer, data); - *static_cast(data) += ": "; + *static_cast(data) << ": "; mlirDiagnosticPrint(diag, printer, data); for (intptr_t i = 0; i < mlirDiagnosticGetNumNotes(diag); i++) { - *static_cast(data) += "\n"; + *static_cast(data) << "\n"; MlirDiagnostic note = mlirDiagnosticGetNote(diag, i); handler(note, data); } @@ -56,7 +59,7 @@ class CollectDiagnosticsToStringScope { MlirContext context; MlirDiagnosticHandlerID handlerID; - std::string errorMessage = ""; + std::ostringstream errorMessage; }; } // namespace python From 6afec89bb95990d91c8a8a6c88c78810024b0d37 Mon Sep 17 00:00:00 2001 From: Nikhil Kalra Date: Mon, 24 Feb 2025 13:21:54 -0800 Subject: [PATCH 03/11] capi to print stack trace on diagnostic --- mlir/include/mlir-c/IR.h | 5 +++++ mlir/lib/CAPI/IR/IR.cpp | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/mlir/include/mlir-c/IR.h b/mlir/include/mlir-c/IR.h index 14ccae650606a..f661e90105704 100644 --- a/mlir/include/mlir-c/IR.h +++ b/mlir/include/mlir-c/IR.h @@ -162,6 +162,11 @@ MLIR_CAPI_EXPORTED bool mlirContextIsRegisteredOperation(MlirContext context, MLIR_CAPI_EXPORTED void mlirContextSetThreadPool(MlirContext context, MlirLlvmThreadPool threadPool); +/// Sets the context to attach the stack trace for the source code location at +/// which a diagnostic is emitted. +MLIR_CAPI_EXPORTED void +mlirContextPrintStackTraceOnDiagnostic(MlirContext context, bool enable); + //===----------------------------------------------------------------------===// // Dialect API. //===----------------------------------------------------------------------===// diff --git a/mlir/lib/CAPI/IR/IR.cpp b/mlir/lib/CAPI/IR/IR.cpp index 999e8cbda1295..2249519ad4eef 100644 --- a/mlir/lib/CAPI/IR/IR.cpp +++ b/mlir/lib/CAPI/IR/IR.cpp @@ -114,6 +114,10 @@ void mlirContextSetThreadPool(MlirContext context, unwrap(context)->setThreadPool(*unwrap(threadPool)); } +void mlirContextPrintStackTraceOnDiagnostic(MlirContext context, bool enable) { + unwrap(context)->printStackTraceOnDiagnostic(enable); +} + //===----------------------------------------------------------------------===// // Dialect API. //===----------------------------------------------------------------------===// From 87cbc6c2624555b145a934bf1d33438af4e8ecc4 Mon Sep 17 00:00:00 2001 From: Nikhil Kalra Date: Mon, 24 Feb 2025 13:22:21 -0800 Subject: [PATCH 04/11] test to ensure diagnostics are logged --- mlir/test/python/ir/diagnostic_handler.py | 14 ++++++++++++++ mlir/test/python/lib/PythonTestModuleNanobind.cpp | 12 ++++++++++++ 2 files changed, 26 insertions(+) diff --git a/mlir/test/python/ir/diagnostic_handler.py b/mlir/test/python/ir/diagnostic_handler.py index d516cda819897..5f6696850682a 100644 --- a/mlir/test/python/ir/diagnostic_handler.py +++ b/mlir/test/python/ir/diagnostic_handler.py @@ -2,6 +2,7 @@ import gc from mlir.ir import * +from mlir._mlir_libs._mlirPythonTestNanobind import test_diagnostics_with_errors_and_notes def run(f): @@ -222,3 +223,16 @@ def callback2(d): # CHECK: CALLBACK2: foobar # CHECK: CALLBACK1: foobar loc.emit_error("foobar") + +# CHECK-LABEL: TEST: testBuiltInDiagnosticsHandler +@run +def testBuiltInDiagnosticsHandler(): + ctx = Context() + + try: + test_diagnostics_with_errors_and_notes(ctx) + except ValueError as e: + # CHECK: created error + # CHECK: MLIRPythonCAPI + print(e) + diff --git a/mlir/test/python/lib/PythonTestModuleNanobind.cpp b/mlir/test/python/lib/PythonTestModuleNanobind.cpp index 99c81eae97a0c..daf3b4602b367 100644 --- a/mlir/test/python/lib/PythonTestModuleNanobind.cpp +++ b/mlir/test/python/lib/PythonTestModuleNanobind.cpp @@ -11,9 +11,12 @@ #include "PythonTestCAPI.h" #include "mlir-c/BuiltinAttributes.h" #include "mlir-c/BuiltinTypes.h" +#include "mlir-c/Diagnostics.h" #include "mlir-c/IR.h" +#include "mlir/Bindings/Python/Diagnostics.h" #include "mlir/Bindings/Python/Nanobind.h" #include "mlir/Bindings/Python/NanobindAdaptors.h" +#include "nanobind/nanobind.h" namespace nb = nanobind; using namespace mlir::python::nanobind_adaptors; @@ -45,6 +48,15 @@ NB_MODULE(_mlirPythonTestNanobind, m) { }, nb::arg("registry")); + m.def("test_diagnostics_with_errors_and_notes", [](MlirContext ctx) { + mlirContextPrintStackTraceOnDiagnostic(ctx, true); + mlir::python::CollectDiagnosticsToStringScope handler(ctx); + + auto loc = mlirLocationUnknownGet(ctx); + mlirEmitError(loc, "created error"); + throw nb::value_error(handler.takeMessage().c_str()); + }); + mlir_attribute_subclass(m, "TestAttr", mlirAttributeIsAPythonTestTestAttribute, mlirPythonTestTestAttributeGetTypeID) From 417ed76166c56aab561a1915df72b3ec5a6a24e9 Mon Sep 17 00:00:00 2001 From: Nikhil Kalra Date: Mon, 24 Feb 2025 16:35:00 -0800 Subject: [PATCH 05/11] formatting --- mlir/test/python/ir/diagnostic_handler.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/mlir/test/python/ir/diagnostic_handler.py b/mlir/test/python/ir/diagnostic_handler.py index 5f6696850682a..d82074690f1a4 100644 --- a/mlir/test/python/ir/diagnostic_handler.py +++ b/mlir/test/python/ir/diagnostic_handler.py @@ -2,7 +2,9 @@ import gc from mlir.ir import * -from mlir._mlir_libs._mlirPythonTestNanobind import test_diagnostics_with_errors_and_notes +from mlir._mlir_libs._mlirPythonTestNanobind import ( + test_diagnostics_with_errors_and_notes, +) def run(f): @@ -224,6 +226,7 @@ def callback2(d): # CHECK: CALLBACK1: foobar loc.emit_error("foobar") + # CHECK-LABEL: TEST: testBuiltInDiagnosticsHandler @run def testBuiltInDiagnosticsHandler(): @@ -235,4 +238,3 @@ def testBuiltInDiagnosticsHandler(): # CHECK: created error # CHECK: MLIRPythonCAPI print(e) - From 9538e20681ea197ce98c8764815367b7e3863ddf Mon Sep 17 00:00:00 2001 From: Nikhil Kalra Date: Tue, 25 Feb 2025 07:31:12 -0800 Subject: [PATCH 06/11] moved test impl --- mlir/test/python/ir/diagnostic_handler.py | 4 ++-- mlir/test/python/lib/PythonTestCAPI.cpp | 8 ++++++++ mlir/test/python/lib/PythonTestCAPI.h | 3 +++ mlir/test/python/lib/PythonTestModuleNanobind.cpp | 3 +-- 4 files changed, 14 insertions(+), 4 deletions(-) diff --git a/mlir/test/python/ir/diagnostic_handler.py b/mlir/test/python/ir/diagnostic_handler.py index d82074690f1a4..6d273e5092e42 100644 --- a/mlir/test/python/ir/diagnostic_handler.py +++ b/mlir/test/python/ir/diagnostic_handler.py @@ -3,7 +3,7 @@ import gc from mlir.ir import * from mlir._mlir_libs._mlirPythonTestNanobind import ( - test_diagnostics_with_errors_and_notes, + test_diagnostics_with_errors_and_notes, ) @@ -236,5 +236,5 @@ def testBuiltInDiagnosticsHandler(): test_diagnostics_with_errors_and_notes(ctx) except ValueError as e: # CHECK: created error - # CHECK: MLIRPythonCAPI + # CHECK: attached note print(e) diff --git a/mlir/test/python/lib/PythonTestCAPI.cpp b/mlir/test/python/lib/PythonTestCAPI.cpp index cb7d7677714fe..ee6a5ae2d9c3d 100644 --- a/mlir/test/python/lib/PythonTestCAPI.cpp +++ b/mlir/test/python/lib/PythonTestCAPI.cpp @@ -11,6 +11,8 @@ #include "mlir-c/BuiltinTypes.h" #include "mlir/CAPI/Registration.h" #include "mlir/CAPI/Wrap.h" +#include "mlir/IR/Diagnostics.h" +#include "mlir/IR/Location.h" MLIR_DEFINE_CAPI_DIALECT_REGISTRATION(PythonTest, python_test, python_test::PythonTestDialect) @@ -42,3 +44,9 @@ MlirTypeID mlirPythonTestTestTypeGetTypeID(void) { bool mlirTypeIsAPythonTestTestTensorValue(MlirValue value) { return mlirTypeIsATensor(wrap(unwrap(value).getType())); } + +void mlirPythonTestEmitDiagnosticWithNote(MlirContext ctx) { + auto diag = + mlir::emitError(unwrap(mlirLocationUnknownGet(ctx)), "created error"); + diag.attachNote() << "attached note"; +} diff --git a/mlir/test/python/lib/PythonTestCAPI.h b/mlir/test/python/lib/PythonTestCAPI.h index 43f8fdcbfae12..5ffd0cbf6201e 100644 --- a/mlir/test/python/lib/PythonTestCAPI.h +++ b/mlir/test/python/lib/PythonTestCAPI.h @@ -10,6 +10,7 @@ #define MLIR_TEST_PYTHON_LIB_PYTHONTESTCAPI_H #include "mlir-c/IR.h" +#include "mlir-c/Support.h" #ifdef __cplusplus extern "C" { @@ -33,6 +34,8 @@ MLIR_CAPI_EXPORTED MlirTypeID mlirPythonTestTestTypeGetTypeID(void); MLIR_CAPI_EXPORTED bool mlirTypeIsAPythonTestTestTensorValue(MlirValue value); +MLIR_CAPI_EXPORTED void mlirPythonTestEmitDiagnosticWithNote(MlirContext ctx); + #ifdef __cplusplus } #endif diff --git a/mlir/test/python/lib/PythonTestModuleNanobind.cpp b/mlir/test/python/lib/PythonTestModuleNanobind.cpp index daf3b4602b367..4cfe84467fb22 100644 --- a/mlir/test/python/lib/PythonTestModuleNanobind.cpp +++ b/mlir/test/python/lib/PythonTestModuleNanobind.cpp @@ -52,8 +52,7 @@ NB_MODULE(_mlirPythonTestNanobind, m) { mlirContextPrintStackTraceOnDiagnostic(ctx, true); mlir::python::CollectDiagnosticsToStringScope handler(ctx); - auto loc = mlirLocationUnknownGet(ctx); - mlirEmitError(loc, "created error"); + mlirPythonTestEmitDiagnosticWithNote(ctx); throw nb::value_error(handler.takeMessage().c_str()); }); From 6d8c1f4edc896172155c8bfb218263ad391fe43b Mon Sep 17 00:00:00 2001 From: Nikhil Kalra Date: Tue, 25 Feb 2025 07:31:41 -0800 Subject: [PATCH 07/11] Revert "capi to print stack trace on diagnostic" This reverts commit 6afec89bb95990d91c8a8a6c88c78810024b0d37. --- mlir/include/mlir-c/IR.h | 5 ----- mlir/lib/CAPI/IR/IR.cpp | 4 ---- 2 files changed, 9 deletions(-) diff --git a/mlir/include/mlir-c/IR.h b/mlir/include/mlir-c/IR.h index f661e90105704..14ccae650606a 100644 --- a/mlir/include/mlir-c/IR.h +++ b/mlir/include/mlir-c/IR.h @@ -162,11 +162,6 @@ MLIR_CAPI_EXPORTED bool mlirContextIsRegisteredOperation(MlirContext context, MLIR_CAPI_EXPORTED void mlirContextSetThreadPool(MlirContext context, MlirLlvmThreadPool threadPool); -/// Sets the context to attach the stack trace for the source code location at -/// which a diagnostic is emitted. -MLIR_CAPI_EXPORTED void -mlirContextPrintStackTraceOnDiagnostic(MlirContext context, bool enable); - //===----------------------------------------------------------------------===// // Dialect API. //===----------------------------------------------------------------------===// diff --git a/mlir/lib/CAPI/IR/IR.cpp b/mlir/lib/CAPI/IR/IR.cpp index 2249519ad4eef..999e8cbda1295 100644 --- a/mlir/lib/CAPI/IR/IR.cpp +++ b/mlir/lib/CAPI/IR/IR.cpp @@ -114,10 +114,6 @@ void mlirContextSetThreadPool(MlirContext context, unwrap(context)->setThreadPool(*unwrap(threadPool)); } -void mlirContextPrintStackTraceOnDiagnostic(MlirContext context, bool enable) { - unwrap(context)->printStackTraceOnDiagnostic(enable); -} - //===----------------------------------------------------------------------===// // Dialect API. //===----------------------------------------------------------------------===// From 877eec9328968718bc780172fa93742a92941547 Mon Sep 17 00:00:00 2001 From: Nikhil Kalra Date: Tue, 25 Feb 2025 07:31:56 -0800 Subject: [PATCH 08/11] remove added api --- mlir/test/python/lib/PythonTestModuleNanobind.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/mlir/test/python/lib/PythonTestModuleNanobind.cpp b/mlir/test/python/lib/PythonTestModuleNanobind.cpp index 4cfe84467fb22..dd0a9f2b66ea6 100644 --- a/mlir/test/python/lib/PythonTestModuleNanobind.cpp +++ b/mlir/test/python/lib/PythonTestModuleNanobind.cpp @@ -49,7 +49,6 @@ NB_MODULE(_mlirPythonTestNanobind, m) { nb::arg("registry")); m.def("test_diagnostics_with_errors_and_notes", [](MlirContext ctx) { - mlirContextPrintStackTraceOnDiagnostic(ctx, true); mlir::python::CollectDiagnosticsToStringScope handler(ctx); mlirPythonTestEmitDiagnosticWithNote(ctx); From 0dd4a25406cd1f8141958101d6b27a99d71ceda9 Mon Sep 17 00:00:00 2001 From: Nikhil Kalra Date: Mon, 10 Mar 2025 10:59:35 -0700 Subject: [PATCH 09/11] use a llvm stream instead --- .../mlir/Bindings/Python/Diagnostics.h | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/mlir/include/mlir/Bindings/Python/Diagnostics.h b/mlir/include/mlir/Bindings/Python/Diagnostics.h index 4f9be844dc1ac..d35e859970ac7 100644 --- a/mlir/include/mlir/Bindings/Python/Diagnostics.h +++ b/mlir/include/mlir/Bindings/Python/Diagnostics.h @@ -11,6 +11,7 @@ #include "mlir-c/Diagnostics.h" #include "mlir-c/IR.h" +#include "llvm/Support/raw_ostream.h" #include #include @@ -25,32 +26,33 @@ namespace python { class CollectDiagnosticsToStringScope { public: explicit CollectDiagnosticsToStringScope(MlirContext ctx) : context(ctx) { - handlerID = mlirContextAttachDiagnosticHandler(ctx, &handler, &errorMessage, - /*deleteUserData=*/nullptr); + handlerID = + mlirContextAttachDiagnosticHandler(ctx, &handler, &messageStream, + /*deleteUserData=*/nullptr); } ~CollectDiagnosticsToStringScope() { mlirContextDetachDiagnosticHandler(context, handlerID); } [[nodiscard]] std::string takeMessage() { - std::ostringstream stream; - std::swap(stream, errorMessage); - return stream.str(); + std::string newMessage; + std::swap(message, newMessage); + return newMessage; } private: static MlirLogicalResult handler(MlirDiagnostic diag, void *data) { auto printer = +[](MlirStringRef message, void *data) { - *static_cast(data) + *static_cast(data) << std::string_view(message.data, message.length); }; MlirLocation loc = mlirDiagnosticGetLocation(diag); - *static_cast(data) << "at "; + *static_cast(data) << "at "; mlirLocationPrint(loc, printer, data); - *static_cast(data) << ": "; + *static_cast(data) << ": "; mlirDiagnosticPrint(diag, printer, data); for (intptr_t i = 0; i < mlirDiagnosticGetNumNotes(diag); i++) { - *static_cast(data) << "\n"; + *static_cast(data) << "\n"; MlirDiagnostic note = mlirDiagnosticGetNote(diag, i); handler(note, data); } @@ -59,7 +61,9 @@ class CollectDiagnosticsToStringScope { MlirContext context; MlirDiagnosticHandlerID handlerID; - std::ostringstream errorMessage; + + std::string message; + llvm::raw_string_ostream messageStream{message}; }; } // namespace python From 5b9bb0cd470a6c01fef6465a10a49d77451f457a Mon Sep 17 00:00:00 2001 From: Nikhil Kalra Date: Mon, 10 Mar 2025 11:00:57 -0700 Subject: [PATCH 10/11] remove unused include --- mlir/include/mlir/Bindings/Python/Diagnostics.h | 1 - 1 file changed, 1 deletion(-) diff --git a/mlir/include/mlir/Bindings/Python/Diagnostics.h b/mlir/include/mlir/Bindings/Python/Diagnostics.h index d35e859970ac7..443d39d172f68 100644 --- a/mlir/include/mlir/Bindings/Python/Diagnostics.h +++ b/mlir/include/mlir/Bindings/Python/Diagnostics.h @@ -15,7 +15,6 @@ #include #include -#include #include namespace mlir { From 2d59eb17f2540bc75b3385b226c4385d6a1ad282 Mon Sep 17 00:00:00 2001 From: Nikhil Kalra Date: Mon, 10 Mar 2025 11:01:51 -0700 Subject: [PATCH 11/11] re-add assert --- mlir/include/mlir/Bindings/Python/Diagnostics.h | 1 + 1 file changed, 1 insertion(+) diff --git a/mlir/include/mlir/Bindings/Python/Diagnostics.h b/mlir/include/mlir/Bindings/Python/Diagnostics.h index 443d39d172f68..167002d561931 100644 --- a/mlir/include/mlir/Bindings/Python/Diagnostics.h +++ b/mlir/include/mlir/Bindings/Python/Diagnostics.h @@ -30,6 +30,7 @@ class CollectDiagnosticsToStringScope { /*deleteUserData=*/nullptr); } ~CollectDiagnosticsToStringScope() { + assert(message.empty() && "unchecked error message"); mlirContextDetachDiagnosticHandler(context, handlerID); }