[PATCH] D79812: [MLIR] Add symbol map to mlir ExecutionEngine

[Eugene Zhulenev via Phabricator via llvm-commits]

ezhulenev added inline comments.


================
Comment at: mlir/lib/ExecutionEngine/ExecutionEngine.cpp:199
     Optional<llvm::CodeGenOpt::Level> jitCodeGenOptLevel,
-    ArrayRef<StringRef> sharedLibPaths, bool enableObjectCache,
-    bool enableGDBNotificationListener, bool enablePerfNotificationListener) {
+    ArrayRef<StringRef> sharedLibPaths, SymbolMap symbolMap,
+    bool enableObjectCache, bool enableGDBNotificationListener,
----------------
mehdi_amini wrote:
> mehdi_amini wrote:
> > Copying an entire DenseMap just to read it isn't ideal. Can we pass is as const ref? 
> > Or better allow the client of the API to not be forced into this narrow contract by using a callback to populate the internal `llvm::orc::SymbolMap` or something like this?
> Also: isn't something can can be done after creation? With a specific API to add symbols?
Done. I changed DenseMap to std::function callback. In theory it can be done later, after the creation of ExecutionEngine, though I think it makes sense to pass these details at the time of "create".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79812