-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[MLIR][ExecutionEngine] Restore internal _mlir_{*} builtins #159941
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
Conversation
@llvm/pr-subscribers-mlir @llvm/pr-subscribers-mlir-execution-engine Author: Balint Cristian (cbalint13) ChangesDuring clang-tidy fixes,
Issue details
Note: Of course one can still use Full diff: https://github.com/llvm/llvm-project/pull/159941.diff 1 Files Affected:
diff --git a/mlir/lib/ExecutionEngine/ExecutionEngine.cpp b/mlir/lib/ExecutionEngine/ExecutionEngine.cpp
index 52162a43aeae3..85775aefd1a02 100644
--- a/mlir/lib/ExecutionEngine/ExecutionEngine.cpp
+++ b/mlir/lib/ExecutionEngine/ExecutionEngine.cpp
@@ -134,7 +134,27 @@ void ExecutionEngine::setupTargetTripleAndDataLayout(Module *llvmModule,
llvmModule->setTargetTriple(tm->getTargetTriple());
}
+static StringRef lookupBuiltinCRunnerNames(StringRef name) {
+ static const llvm::StringMap<StringRef> nameMap = {
+ {"alloc", "mlirAlloc"},
+ {"aligned_alloc", "mlirAlignedAlloc"},
+ {"malloc", "mlirAlloc"},
+ {"free", "mlirFree"},
+ {"aligned_free", "mlirAlignedFree"}};
+
+ auto it = nameMap.find(name);
+ if (it != nameMap.end()) {
+ return it->second;
+ }
+
+ return {};
+}
+
static std::string makePackedFunctionName(StringRef name) {
+ auto cfuncName = lookupBuiltinCRunnerNames(name);
+ if (!cfuncName.empty())
+ return cfuncName.str();
+
return "_mlir_" + name.str();
}
|
Cc @joker-eph , @ftynse (authors / code blame list) |
Hmm, this sort of dynamic renaming makes the overall behavior of a system harder to follow, all just to keep the linter happy... Can we rather revert the offending commit + add configuration that prevents it from complaining about these names? |
+1 to revert + a test using the symbols otherwise this is dead. |
As per review requests:
|
Not sure it is worth, the issue would persist across (too) many older versions. |
During clang-tidy fixes,
_mlir_{alloc,aligned_alloc,free,aligned_free}
were lost, see the commit: c599650mlir{Alloc,AlignedAlloc,Free,AlignedFree}
are dangling declarations.Issue details
Note: Of course one can still use
use-generic-functions=true
with his own custom shared library, but the scope of this PR is to restore the functionality back to simply whatlibmlir_c_runner_utils.so
already ships as builtins.