Fix OrcError build with modules enabled.

[vsapsai]

Summary

Bot http://green.lab.llvm.org/green/job/clang-stage2-Rthinlto/ fails
with an error

error: 'llvm/IR/Attributes.inc' file not found

while building OrcError. It happens because OrcError.cpp depends on
'LLVM_ExecutionEngine' module through including
"llvm/ExecutionEngine/Orc/OrcError.h". And `LLVM_ExecutionEngine'
depends on 'LLVM_intrinsic_gen' through ExecutionEngine.h including
"llvm/IR/Module.h". But we fail to build 'LLVM_intrinsic_gen' because
'llvm/IR/Attributes.inc' is missing. Note that this problem doesn't
happen in a non-modular build because OrcError files don't try to
include .inc files transitively.

Fix by making the target LLVMOrcError depend on intrinsics_gen the same
way LLVMOrcJIT does.

rdar://problem/56377508

[vsapsai]

A better solution might be creating a proper module for OrcError but given the discussion in https://reviews.llvm.org/D68732 I'm not sure what is the future of OrcError library and if it is worth spending time on tweaking a module map.

[beanz]

This confuses me. LLVMOrcError only directly depends on LLVMSupport. It doesn't include or need anything from ExecutionEngine.h.

The real issue is, why is that getting pulled in?

[beanz]

Adding exclude header "ExecutionEngine/Orc/OrcError.h" to the module specification for LLVM_ExecutionEngine seems like a more appropriate fix.

[vsapsai]

In D69817#1734427, @beanz wrote:

Adding exclude header "ExecutionEngine/Orc/OrcError.h" to the module specification for LLVM_ExecutionEngine seems like a more appropriate fix.

That's not enough. RPCError.cpp includes "llvm/ExecutionEngine/Orc/RPC/RPCUtils.h" which is a part of LLVM_ExecutionEngine module and we still have unsatisfied dependency LLVM_ExecutionEngine -> LLVM_intrinsic_gen. I agree it is better to have a proper OrcError module but at the moment I don't see a clear set of headers constituting such a module. We can add together error and RPC headers but to me this combination looks weird. Though I haven't worked with JIT/Orc/RPC code and maybe that's the most natural composition.

[beanz]

RPCUtils.h and RPCSerialization.h can both also be excluded.

[vsapsai]

In D69817#1734651, @beanz wrote:

RPCUtils.h and RPCSerialization.h can both also be excluded.

I don't think that's the right fix as it moves a module map from representing a project structure towards having a random set of headers. But even after excluding 3 headers there are errors

llvm-project/llvm/include/llvm/Support/Error.h:331:10: error: no matching conversion for functional-style cast from 'typename unique_if<JITSymbolNotFound>::unique_single' (aka 'unique_ptr<llvm::orc::JITSymbolNotFound>') to 'llvm::Error'

return Error(std::make_unique<ErrT>(std::forward<ArgTs>(Args)...));
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

host-compiler/bin/../include/c++/v1/memory:3003:32: error: definition of 'JITSymbolNotFound' must be imported from module 'LLVM_ExecutionEngine.Orc.CompileOnDemandLayer' before it is required

return unique_ptr<_Tp>(new _Tp(_VSTD::forward<_Args>(__args)...));
                           ^

Have you seen these errors before? The second error implies a module map with excluded headers doesn't work.

[beanz]

In D69817#1734876, @vsapsai wrote:

I don't think that's the right fix as it moves a module map from representing a project structure towards having a random set of headers. But even after excluding 3 headers there are errors

ExecutionEngine isn't like the rest of LLVM's project structure. It should not be treated as a single module component. We can debate whether or not that is appropriate in LLVM conventions, but it is the way the code is structured. The module map is not representative of the actual structure of the code under the ExecutionEngine header directory.

llvm-project/llvm/include/llvm/Support/Error.h:331:10: error: no matching conversion for functional-style cast from 'typename unique_if<JITSymbolNotFound>::unique_single' (aka 'unique_ptr<llvm::orc::JITSymbolNotFound>') to 'llvm::Error'

return Error(std::make_unique<ErrT>(std::forward<ArgTs>(Args)...));
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

There isn't enough context on this error for me to understand what is causing it, but the problem is in the module map.

Have you seen these errors before? The second error implies a module map with excluded headers doesn't work.

The ExecutionEngine module map has a bunch of excludes already because of similar errors in the past. The ORC error and RPC related headers only rely on LLVMSupport, and can be used independently of the rest of ExecutionEngine. This is *very* important because it allows building remote JIT clients that don't depend on the LLVM IR, or Target libraries. The problem you're experiencing is the result of the module map not appropriately representing the orc error and RPC headers as separable parts from the ExecutionEngine module.

[lhames]

I think Chris is right. This isn’t an ideal breakdown of the library, but it’s a step in the right direction: OrcError should should only depend on Support (not the rest of ExecutionEngine), and ExecutionEngine should depend on OrcError.

I’ll look at this tomorrow and see if I can figure out how to structure the modules to match that layering.

[lhames]

I couldn’t reproduce this -DLLVM_ENABLE_MODULES=On and my system compiler. I’ll try a 2-stage build and see if I have any luck with that.

Volodymyr — Can you provide more context around this error:

host-compiler/bin/../include/c++/v1/memory:3003:32: error: definition of 'JITSymbolNotFound' must be imported from module 'LLVM_ExecutionEngine.Orc.CompileOnDemandLayer' before it is required

I’m not quite sure how to parse it, but if it’s saying that CompileOnDemandLayer needs to import OrcError in modules builds then we should just do that (it’s a correct and expected requirement).

If it’s implying that JITSymbolNotFound is being provided *by* CompileOnDemandLayer then that’s a modules bug of some kind.

[vsapsai]

The error with the notes is

In module 'std' imported from llvm-project/llvm/lib/ExecutionEngine/Orc/OrcMCJITReplacement.h:40:
host-compiler/bin/../include/c++/v1/memory:3003:32: error: definition of 'JITSymbolNotFound' must be imported from module 'LLVM_ExecutionEngine.Orc.CompileOnDemandLayer' before it is required
    return unique_ptr<_Tp>(new _Tp(_VSTD::forward<_Args>(__args)...));
                               ^
llvm-project/llvm/include/llvm/Support/Error.h:331:21: note: in instantiation of function template specialization 'std::__1::make_unique<llvm::orc::JITSymbolNotFound, const std::__1::basic_string<char> &>' requested here
  return Error(std::make_unique<ErrT>(std::forward<ArgTs>(Args)...));
                    ^
llvm-project/llvm/include/llvm/ExecutionEngine/Orc/ExecutionUtils.h:130:18: note: in instantiation of function template specialization 'llvm::make_error<llvm::orc::JITSymbolNotFound, const std::__1::basic_string<char> &>' requested here
          return make_error<JITSymbolNotFound>(CtorDtorName);
                 ^
llvm-project/llvm/lib/ExecutionEngine/Orc/OrcMCJITReplacement.cpp:132:19: note: in instantiation of member function 'llvm::orc::LegacyCtorDtorRunner<llvm::orc::LazyEmittingLayer<llvm::orc::LegacyIRCompileLayer<llvm::orc::LegacyRTDyldObjectLinkingLayer, llvm::orc::SimpleCompiler> > >::runViaLayer' requested here
                 .runViaLayer(LazyEmitLayer));
                  ^
llvm-project/llvm/include/llvm/ExecutionEngine/Orc/OrcError.h:55:7: note: previous definition is here
class JITSymbolNotFound : public ErrorInfo<JITSymbolNotFound> {
      ^

To me it looks like symbols from OrcError.h aren't attributed to a single module, so they end up in multiple places and clang fails to reconcile all these multiple copies.

As for reproducing, I was using python zorg/zorg/jenkins/monorepo_build.py clang build --thinlto '--projects=clang;compiler-rt;libcxx' --cmake-flag=-DLIBCXX_ENABLE_SHARED=OFF --cmake-flag=-DLIBCXX_ENABLE_STATIC=OFF --cmake-flag=-DLIBCXX_INCLUDE_TESTS=OFF

[vsapsai]

The bot is green now.

llvm/lib/ExecutionEngine/OrcError/CMakeLists.txt

 add_llvm_library(LLVMOrcError
   OrcError.cpp
   RPCError.cpp
   ADDITIONAL_HEADER_DIRS
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/ExecutionEngine/Orc
+
+  DEPENDS
+  intrinsics_gen
   )