[Mlir-commits] [mlir] [MLIR][JitRunner] Correctly register symbol map (PR #90381)

[llvmlistbot at llvm.org]

Menooker wrote:

> > In `mlir/lib/ExecutionEngine/JitRunner.cpp` I can't find a use of `runtimeSymbolMap`. Could you please point out where it registers and uses `runtimeSymbolMap`?
> 
> The commit you mention calls `engine->registerSymbols(runtimeSymbolMap)` [here](https://github.com/llvm/llvm-project/blob/0b3841eb97/mlir/lib/ExecutionEngine/ExecutionEngine.cpp#L399).
> 
> From memories, the intention of the commit was to preserve semtantics; let's see if I can argue how that was achieved: `runtimeSymbolMap` is a function that uses `exportSymbols` in its capture both before and after the commit. In both versions, the capture is populated with the init functions that are extracted via `getAddressOfSymbol` of all files in `libPaths`/`sharedLibPaths`. That vector, in turn, was populated by command line options of the _JiT runner_ before the commit and by the options of the _execution engine_. Finally, in the JiT runner, the paths given as CLI arguments are used to construct the paths passed as options to the execution engine by the commit, so I'd expect the behavior to be exactly the same.
> 
> (What I haven't checked yet is whether the code has been modified in the meantime.)

Thanks for the details of the context of your commit! The problem I am trying to fix in this PR is a bit different that what you discussed, I think. `engine->registerSymbols(runtimeSymbolMap)`  is indeed called in `ExecutionEngine::create`. However, `mlir::JitRunnerMain` has its own `runtimeSymbolMap` passed by the argument `JitRunnerConfig config`. But if you track the dataflow of `JitRunnerConfig::runtimeSymbolMap`, you will find that it is never passed to an `ExecutionEngine`. So `JitRunnerConfig::runtimeSymbolMap` given by the user is never used in `mlir::JitRunnerMain`.

I think it'd better not to leave this API (JitRunnerConfig::runtimeSymbolMap) malfunctioning. It is confusing to users like me, to pass a symbol map, and seeing it not working. I would suggest either fixing the API (by this PR), or removing this field in JitRunnerConfig (as suggested by @joker-eph , it is being deprecated).

https://github.com/llvm/llvm-project/pull/90381