[HAL] Add option to disable executable linking #19028
Conversation
There was a problem hiding this comment.
hmm bool flags with "disable" or "enable" in the name are odd (I don't love that they have spread in codegen/flow as much as they have) - a bool true implies enable and bool false implies disable. The annoying part of iree-opt is that pass names are flags so we can't make this what would be best (--iree-hal-link-executables=true to link by default and --iree-hal-link-executables=false to not link as an override).
We can rename the pass though - there's LinkExecutablesPass and LinkTargetExecutablesPass - we could make that LinkAllExecutablesPass and LinkTargetExecutablesPass so the pass name becomes --iree-hal-link-all-executables and then we have the iree-hal-link-executables flag name available for use.
I've meant to do this with the other passes of the same style (SerializeExecutablesPass -> SerializeAllExecutablesPass, TranslateExecutablesPass -> TranslateAllExecutablesPass) since I originally split them up so it'd be good cleanup. If you can find/replace those here then we can use that flag right away, otherwise I can send a PR to do that.
|
Yeah I wasn't sure what to name it so I just picked something. I can change the |
|
if you can do em all that'd be nice - just 3 find/replace instead of 1 (no need to change file names, just pass .td/C++ class name). |
This is useful because without disabling linking, `--iree-hal-dump-executable-binaries-to` and `--iree-hal-dump-executable-intermediates-to` will dump a single large linked file rather than one file per executable. This is expected to only ever be used for development/debugging purposes.
qedawkins
force-pushed
the
disable_linking_flag
branch
from
fc794a8 to
fee1701
Compare
| @@ -475,8 +485,9 @@ void buildHALTransformPassPipeline(OpPassManager &passManager, | |||
| // example, the LLVM AOT backend may combine all executable targets for the | |||
| // same architecture into a single executable and link it as a shared | |||
| // library. | |||
| if (transformOptions.linkExecutables) { | |||
| passManager.addPass(IREE::HAL::createLinkExecutablesPass({targetRegistry})); | |||
| if (transformOptions.linkExecutables && clLinkExecutables) { | |||
There was a problem hiding this comment.
It is strange to me that this is && but if it isn't then to disable linking we'd have to plumb through another flag for the transform option and disable that too. I think it's fine because linking should pretty much always be on but if there are any other suggestions.
|
Ok I accidentally turned off linking by default, but it looks like there are some test failures with it off: https://github.com/iree-org/iree/actions/runs/11744495978/job/32719757104?pr=19028 May want to look into that at some point. |
This is useful because without disabling linking, `--iree-hal-dump-executable-binaries-to` and `--iree-hal-dump-executable-intermediates-to` will dump a single large linked file rather than one file per executable. This is expected to only ever be used for development/debugging purposes. Signed-off-by: Giacomo Serafini <179146510+giacs-epic@users.noreply.github.com>
This is useful because without disabling linking,
--iree-hal-dump-executable-binaries-toand--iree-hal-dump-executable-intermediates-towill dump a single large linked file rather than one file per executable.This is expected to only ever be used for development/debugging purposes.