[PATCH] D107087: Fix SectionMemoryManager deconstruction error with MSVC

Wed May 17 05:35:46 PDT 2023

sgraenitz requested changes to this revision.
sgraenitz added a comment.
This revision now requires changes to proceed.

This issues popped up with GCC 12 as well after https://github.com/llvm/llvm-project/commit/47f5c54f997a59bb2c65abe6b8b811f6e7553456

However, I don't think this patch is ready yet. I see two options:
(a) Keep the current approach to break the interface, fix all users of `SectionMemoryManager` (in MLIR <https://github.com/llvm/llvm-project/blob/release/16.x/mlir/include/mlir/ExecutionEngine/ExecutionEngine.h#L80> for example, but I am not sure that's all) and maybe add a release note
(b) Distinguish between owned and non-owned `MemoryMapper` and fix only the `DefaultMMapperInstance` case that causes the Kaleidoscope issue

IMHO option (b) is the better solution, because it doesn't break the API and clients can get away without the `shared_ptr`. What do you think?

================
Comment at: llvm/lib/ExecutionEngine/SectionMemoryManager.cpp:16
 #include "llvm/Config/config.h"
+#include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/MathExtras.h"
----------------
We don't need this anymore

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107087/new/

https://reviews.llvm.org/D107087