Revert "[mlir][ExecutionEngine] Add support for global constructors and destructors" #78164
Conversation
|
@llvm/pr-subscribers-mlir-execution-engine @llvm/pr-subscribers-mlir Author: Cullen Rhodes (c-rhodes) Changesthis is causing test failures for me on AArch64 linux, hitting the following assert: Seeing the same in buildbot as well, e.g. This seems related to #71963 which there's an open PR #71968 for, I've tested that patch but it doesn't fix the issue. Reverts llvm/llvm-project#78070 Full diff: https://github.com/llvm/llvm-project/pull/78164.diff 2 Files Affected:
diff --git a/mlir/lib/ExecutionEngine/ExecutionEngine.cpp b/mlir/lib/ExecutionEngine/ExecutionEngine.cpp
index d4ad2da045e2c4..267ec236d3fd59 100644
--- a/mlir/lib/ExecutionEngine/ExecutionEngine.cpp
+++ b/mlir/lib/ExecutionEngine/ExecutionEngine.cpp
@@ -219,9 +219,6 @@ ExecutionEngine::ExecutionEngine(bool enableObjectDump,
}
ExecutionEngine::~ExecutionEngine() {
- // Execute the global destructors from the module being processed.
- if (jit)
- llvm::consumeError(jit->deinitialize(jit->getMainJITDylib()));
// Run all dynamic library destroy callbacks to prepare for the shutdown.
for (LibraryDestroyFn destroy : destroyFns)
destroy();
@@ -399,9 +396,6 @@ ExecutionEngine::create(Operation *m, const ExecutionEngineOptions &options,
};
engine->registerSymbols(runtimeSymbolMap);
- // Execute the global constructors from the module being processed.
- cantFail(engine->jit->initialize(engine->jit->getMainJITDylib()));
-
return std::move(engine);
}
diff --git a/mlir/test/mlir-cpu-runner/global-constructors.mlir b/mlir/test/mlir-cpu-runner/global-constructors.mlir
deleted file mode 100644
index 3ebcaeccc83e32..00000000000000
--- a/mlir/test/mlir-cpu-runner/global-constructors.mlir
+++ /dev/null
@@ -1,32 +0,0 @@
-// RUN: mlir-cpu-runner %s -e entry -entry-point-result=void \
-// RUN: -shared-libs=%mlir_c_runner_utils | \
-// RUN: FileCheck %s
-
-// Test that the `ctor` executes before `entry` and that `dtor` executes last.
-module {
- llvm.func @printNewline()
- llvm.func @printI64(i64)
- llvm.mlir.global_ctors {ctors = [@ctor], priorities = [0 : i32]}
- llvm.mlir.global_dtors {dtors = [@dtor], priorities = [0 : i32]}
- llvm.func @ctor() {
- %0 = llvm.mlir.constant(1 : i64) : i64
- llvm.call @printI64(%0) : (i64) -> ()
- llvm.call @printNewline() : () -> ()
- // CHECK: 1
- llvm.return
- }
- llvm.func @entry() {
- %0 = llvm.mlir.constant(2 : i64) : i64
- llvm.call @printI64(%0) : (i64) -> ()
- llvm.call @printNewline() : () -> ()
- // CHECK: 2
- llvm.return
- }
- llvm.func @dtor() {
- %0 = llvm.mlir.constant(3 : i64) : i64
- llvm.call @printI64(%0) : (i64) -> ()
- llvm.call @printNewline() : () -> ()
- // CHECK: 3
- llvm.return
- }
-}
|
…nd destructors" (llvm#78164) this is causing test failures on AArch64 linux, hitting the following assert: # | mlir-cpu-runner: /home/culrho01/llvm-project/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp:519: void llvm::RuntimeDyldELF::resolveAArch64Relocation(const SectionEntry &, uint64_t, uint64_t, uint32_t, int64_t): Assertion `isInt<33>(Result) && "overflow check failed for relocation"' failed. Seeing the same in buildbot as well, e.g. https://lab.llvm.org/buildbot/#/builders/179/builds/9094/steps/12/logs/FAIL__MLIR__sparse_codegen_dim_mlir Reverts llvm#78070
Just saw this - when you tested with that patch, did you do something to ensure that |
No I didn't, just cherry-picked on top of the patch this reverts. That could explain it. I doubt I'll look into it as it's not immediately obvious how to ensure that and I can't dig into it at the moment, but could be helpful if someone wants to, thanks for replying. |
Understood. If you (or someone else) does want to try this, then after applying the changes in PR #71968, in https://github.com/llvm/llvm-project/pull/71968/files#diff-e576acec9556f141c95c5c7cb5f7f0caaa71b76ab65f67f95d88a169f933d720R110, change I would also mention that the issue that the test shows up is a latent bug, so just removing support for global constructors and destructors doesn't fix the issue - it just removes a scenario that happens to trigger it. |
this is causing test failures for me on AArch64 linux, hitting the following assert:
Seeing the same in buildbot as well, e.g.
https://lab.llvm.org/buildbot/#/builders/179/builds/9094/steps/12/logs/FAIL__MLIR__sparse_codegen_dim_mlir
This seems related to #71963 which there's an open PR #71968 for, I've tested that patch but it doesn't fix the issue.
Reverts #78070