[mlir][ExecutionEngine] propagate errors in mlirExecutionEngineCreate

[makslevental]

No description provided.

[llvmbot]

@llvm/pr-subscribers-mlir-execution-engine

Author: Maksim Levental (makslevental)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/170592.diff

1 Files Affected:

• (modified) mlir/lib/CAPI/ExecutionEngine/ExecutionEngine.cpp (+7-3)

diff --git a/mlir/lib/CAPI/ExecutionEngine/ExecutionEngine.cpp b/mlir/lib/CAPI/ExecutionEngine/ExecutionEngine.cpp
index 2dbb993b1640f..ab53657577aec 100644
--- a/mlir/lib/CAPI/ExecutionEngine/ExecutionEngine.cpp
+++ b/mlir/lib/CAPI/ExecutionEngine/ExecutionEngine.cpp
@@ -38,12 +38,15 @@ mlirExecutionEngineCreate(MlirModule op, int optLevel, int numPaths,
 
   auto tmBuilderOrError = llvm::orc::JITTargetMachineBuilder::detectHost();
   if (!tmBuilderOrError) {
-    llvm::errs() << "Failed to create a JITTargetMachineBuilder for the host\n";
+    llvm::errs()
+        << "Failed to create a JITTargetMachineBuilder for the host because: "
+        << tmBuilderOrError.takeError() << "\n";
     return MlirExecutionEngine{nullptr};
   }
   auto tmOrError = tmBuilderOrError->createTargetMachine();
   if (!tmOrError) {
-    llvm::errs() << "Failed to create a TargetMachine for the host\n";
+    llvm::errs() << "Failed to create a TargetMachine for the host because: "
+                 << tmOrError.takeError() << "\n";
     return MlirExecutionEngine{nullptr};
   }
 
@@ -62,7 +65,8 @@ mlirExecutionEngineCreate(MlirModule op, int optLevel, int numPaths,
   jitOptions.enableObjectDump = enableObjectDump;
   auto jitOrError = ExecutionEngine::create(unwrap(op), jitOptions);
   if (!jitOrError) {
-    consumeError(jitOrError.takeError());
+    llvm::errs() << "Failed to create an ExecutionEngine because: "
+                 << jitOrError.takeError() << "\n";
     return MlirExecutionEngine{nullptr};
   }
   return wrap(jitOrError->release());

[llvmbot]

@llvm/pr-subscribers-mlir

Author: Maksim Levental (makslevental)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/170592.diff

1 Files Affected:

• (modified) mlir/lib/CAPI/ExecutionEngine/ExecutionEngine.cpp (+7-3)

diff --git a/mlir/lib/CAPI/ExecutionEngine/ExecutionEngine.cpp b/mlir/lib/CAPI/ExecutionEngine/ExecutionEngine.cpp
index 2dbb993b1640f..ab53657577aec 100644
--- a/mlir/lib/CAPI/ExecutionEngine/ExecutionEngine.cpp
+++ b/mlir/lib/CAPI/ExecutionEngine/ExecutionEngine.cpp
@@ -38,12 +38,15 @@ mlirExecutionEngineCreate(MlirModule op, int optLevel, int numPaths,
 
   auto tmBuilderOrError = llvm::orc::JITTargetMachineBuilder::detectHost();
   if (!tmBuilderOrError) {
-    llvm::errs() << "Failed to create a JITTargetMachineBuilder for the host\n";
+    llvm::errs()
+        << "Failed to create a JITTargetMachineBuilder for the host because: "
+        << tmBuilderOrError.takeError() << "\n";
     return MlirExecutionEngine{nullptr};
   }
   auto tmOrError = tmBuilderOrError->createTargetMachine();
   if (!tmOrError) {
-    llvm::errs() << "Failed to create a TargetMachine for the host\n";
+    llvm::errs() << "Failed to create a TargetMachine for the host because: "
+                 << tmOrError.takeError() << "\n";
     return MlirExecutionEngine{nullptr};
   }
 
@@ -62,7 +65,8 @@ mlirExecutionEngineCreate(MlirModule op, int optLevel, int numPaths,
   jitOptions.enableObjectDump = enableObjectDump;
   auto jitOrError = ExecutionEngine::create(unwrap(op), jitOptions);
   if (!jitOrError) {
-    consumeError(jitOrError.takeError());
+    llvm::errs() << "Failed to create an ExecutionEngine because: "
+                 << jitOrError.takeError() << "\n";
     return MlirExecutionEngine{nullptr};
   }
   return wrap(jitOrError->release());

[github-actions]

🐧 Linux x64 Test Results

• 7179 tests passed
• 596 tests skipped

✅ The build succeeded and all tests passed.

[kuhar]

Is this something we could have lit tests for?

[makslevental]

Is this something we could have lit tests for?

Not lit but I can try adding Python tests (there's an existing Python test for the 3rd one)

[joker-eph]

It's preexisting on the other codepath you're changing, but I'm surprised we diagnose errors unconditionally through llvm::errs() ; why is this done in these cases? Are there other APIs we do this?

[makslevental]

not sure - before my time https://reviews.llvm.org/D102551

[makslevental]

Is this something we could have lit tests for?

I failed to figure out a way to trigger each of the other fails (downstream I stumbled upon this by having an unconventional host/target triple relationship). Best I could do was add a lit check line for the current/existing test (which will function the same for the new reports).