[PATCH] D95747: Fix modules build for LLVMOrcShared

Raphael Isemann via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 1 05:26:24 PST 2021


teemperor added a comment.

Thanks for working on this! I think @lhames is the right person for any code restructuring, but I can review all the modulemap stuff.



================
Comment at: llvm/include/llvm/module.modulemap:196
   exclude header "ExecutionEngine/Orc/RPC/RPCSerialization.h"
   exclude header "ExecutionEngine/Orc/RPC/RawByteChannel.h"
 
----------------
sgraenitz wrote:
> These moved to OrcShared, but I don't get errors as is. So, can we just remove them?
I don't think Clang reports missing excluded headers. I think the paths here should be update as I *believe* Clang otherwise compiles the headers twice into each module.


================
Comment at: llvm/include/llvm/module.modulemap:212
 // be removed in the future.
 module LLVM_OrcSupport {
   requires cplusplus
----------------
sgraenitz wrote:
> Is OrcSupport meant as the lib we call OrcShared today?
I think all the modules we have here are *supposed* to represent a specific library, but in some situations (like here) it's just a group of headers that at least allow us to compile everything as a module. If putting all the `Orc/Shared` headers into their own module works with the module build, then we can also rename this to `LLVM_OrcShared` and use an umbrella like in `LLVM_DebugInfo_CodeView` above.

I think it would be even better if we could split up the different headers into modules that match the specific libraries, e.g. `LLVM_OrcJIT`, `LLVM_OrcTargetProcess`, `LLVM_OrcShared` and a `LLVM_ExecutionEngine` module (the last one only contains the headers directly in `ExecutionEngine`, but given the comments above I'm not sure if we can modularize them).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95747



More information about the llvm-commits mailing list