[PATCH] D159085: [lli] Make sure the exported __chkstk functions are included when exporting them

Fri Sep 1 08:41:28 PDT 2023

mati865 added inline comments.

================
Comment at: llvm/tools/lli/lli.cpp:1204
 // Therefore, manually export the relevant symbol from lli, to let it be
 // found at runtime during tests.
 //
----------------
mstorsjo wrote:
> mati865 wrote:
> > Should we update this comment?
> > This workaround is also required for linking to shared libraries.
> Do you mean if building an user application that does JIT, against the LLVM libraries? Yeah, the same issue applies there as well (regardless of whether it's linked shared or statically) - doing it within the `lli` executable just fixes it for the executable and none of the lib users. There's a TODO down below that this really should be handled by ORC.
> 
> And ideally we shouldn't just be exporting this function, ideally ORC should find the right `libclang_rt.builtins*.a` and link in that, it's just that the chkstk functions are the most blatant examples of it.
> 
> Put another way - I wasn't really planning on working on the JIT stuff, I just wanted to make `check-llvm` run successfully in mingw builds; first I tried just mark most of the JIT tests unsupported for mingw configs, but @lhames made and suggested some fixes that brought us to this point instead. The root issue isn't really solved but the tests at least run.
Sorry, I've put this comment in the wrong place, I meant lines 1206-1207.

> Do you mean if building an user application that does JIT, against the LLVM libraries? Yeah, the same issue applies there as well (regardless of whether it's linked shared or statically) - doing it within the lli executable just fixes it for the executable and none of the lib users. There's a TODO down below that this really should be handled by ORC.

I mean without this workaround `lli` is not buildable when linking to shared LLVM libs (IIUC this is what happened at MSYS2). I don't expect fixing ORC here.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159085