[llvm-dev] Orc JIT Layering

[Stefan Gränitz via llvm-dev]

Hi David

Thanks for the details. Yes, the layering issue is something we should
take care of soon. It also makes trouble for the modules build (see:
https://reviews.llvm.org/D95747).

I think we should split up the JITSymbol.h and move JITTargetAddress
into OrcShared. What remains would be the JITSymbol class. Moving this
one to Support sounds like a nice solution to me.

However, we have a similar situation with the GDB JIT interface
declarations. They should have their own header, yes, but where would we
put it? Support too? Not sure about it. Having the definition only in
OrcTargetProcess would be acceptable IMHO. The only alternative seems to
be an entirely new library (as discussed in the review
https://reviews.llvm.org/D97339).

What do you think?

@Peter, @Nico: I've noticed your post-review comments. I will have a
look tonight (~10h from now).

Best,
Stefan

On 03/03/2021 04:57, David Blaikie wrote:
> Seems one of the latest Orc changes
> ( https://github.com/llvm/llvm-project/commit/99a6d003edbe97fcb94854547276ffad3382ec1d
> <https://github.com/llvm/llvm-project/commit/99a6d003edbe97fcb94854547276ffad3382ec1d>
> ) while not itself changing/breaking the layering in LLVM's own build,
> it has revealed some pre-existing problems with the layering that we'd
> worked around at Google in a way that isn't viable after this recent
> change.
>
> One immediate/easily observed issue:
> lib/ExecutionEngine's CMakeLists.txt says it depends on
> OrcTargetProcess, but OrcTargetProcess includes
> lib/ExecutionEngine/JITSymbol.h
>
> The only common dependency for all the uses of JITSymbol.h seems to be
> llvm/Support (ie: without introducing new dependencies or new
> libraries, JITSymbol.h would need to be moved to llvm/Support to fix
> this particular dependency cycle/issue)
>
> We do have a bunch of other workarounds for Orc layering in the Google
> internal build system too - so perhaps I can enumerate some/all of the
> issues here, as it might be best to take a holistic approach to fixing
> these issues.
>
> Let's see what I can document/figure out... 
>
> ExecutionEngine/Orc -> ExecutionEngine
> ExecutionEngine/Interpreter -> ExecutionEngine
> ExecutionEngine/RuntimeDyld -> ExecutionEngine
> ExecutionEngine/IntelJITEvents -> ExecutionEngine
> ExecutionEngine/OProfileJIT -> ExecutionEngine
> ExecutionEngine/PerfJITEvents -> ExecutionEngine
> ExecutionEngine/MCJIT -> ExecutionEngine
>
> And there's actually no #includes in ExecutionEngine that reference
> those libraries, so that's pretty good.
>
> It is this CMakeLists.txt dependency from ExecutionEngine to
> OrcTargetProcess. Which happens without a #include:
>
> $ grep -r "void __jit_debug_register_code" llvm/
>
> llvm/lib/ExecutionEngine/GDBRegistrationListener.cpp:  extern "C"
> *void __jit_debug_register_code*();
>
> llvm/lib/ExecutionEngine/Orc/TargetProcess/JITLoaderGDB.cpp:LLVM_ATTRIBUTE_NOINLINE
> *void __jit_debug_register_code*() {
>
>
> Would be better if this wasn't declared arbitrarily (instead, if it
> was declared in a header and defined as usual, the circular dependence
> would be more clear, I think?) - but either way, the circular
> dependency needs to be fixed.
>
> - Dave
>
>
>
-- 
https://flowcrypt.com/pub/stefan.graenitz@gmail.com

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210303/94023fba/attachment.html>