Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[mlir][transform] Improve error when merging of modules fails. #69331

Merged

Conversation

ingomueller-net
Copy link
Contributor

This resolved #69112.

@llvmbot llvmbot added the mlir label Oct 17, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 17, 2023

@llvm/pr-subscribers-mlir

Author: Ingo Müller (ingomueller-net)

Changes

This resolved #69112.


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

1 Files Affected:

  • (modified) mlir/lib/Dialect/Transform/Transforms/TransformInterpreterUtils.cpp (+2-2)
diff --git a/mlir/lib/Dialect/Transform/Transforms/TransformInterpreterUtils.cpp b/mlir/lib/Dialect/Transform/Transforms/TransformInterpreterUtils.cpp
index 41feffffaf97b3f..e0bde020daf7c6f 100644
--- a/mlir/lib/Dialect/Transform/Transforms/TransformInterpreterUtils.cpp
+++ b/mlir/lib/Dialect/Transform/Transforms/TransformInterpreterUtils.cpp
@@ -176,8 +176,8 @@ LogicalResult transform::detail::assembleTransformLibraryFromPaths(
     for (OwningOpRef<ModuleOp> &parsedLibrary : parsedLibraries) {
       if (failed(transform::detail::mergeSymbolsInto(
               mergedParsedLibraries.get(), std::move(parsedLibrary))))
-        return mergedParsedLibraries->emitError()
-               << "failed to verify merged transform module";
+        return parsedLibrary->emitError()
+               << "failed to merge symbols into shared library module";
     }
   }
 

@ftynse
Copy link
Member

ftynse commented Oct 17, 2023

I'm not sure this fixes the issue I originally raised. My point was that we shouldn't report two diagnostics for the same error, not the location or the contents of the second diagnostic.

@ingomueller-net ingomueller-net merged commit f07718b into llvm:main Oct 24, 2023
3 checks passed
@ingomueller-net ingomueller-net deleted the transform-interpreter-error-message branch October 24, 2023 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should mergedParsedLibraries always emit an error ?
4 participants