Cyclic module dependency [Re: [llvm] r357201 - Temporarily revert "SafepointIRVerifier port to new Pass Manager"]

Sat Mar 30 07:49:32 PDT 2019


On 3/30/19 5:46 PM, Fedor Sergeev wrote:
>
>
> On 3/29/19 11:29 PM, Adrian Prantl wrote:
>> When you compile with -fmodules without local submodule visibility 
>> (which is a configuration we support in LLVM), including any header 
>> from one top-level module will implicitly also import all headers in 
>> that module, even ones that are not explicitly #included in the 
>> source file. So any cyclic header dependency between any two headers 
>> in two modules will be a problem, even if the two headers don't 
>> directly include each other.
> Well, the *theory* of cyclic dependencies is well understood here :)
> However, the practical implications of that are absolutely unclear.
>
> The answer to this question would be nice to have:
>   "Why Verifier.h does not cause a similar build issue?"
>
> I can only guess that it is due to the following line in 
> module.modulemap:
>     module IR_Verifier { header "IR/Verifier.h" export * }
>
> Thus I assume that to fix our build failure I should add 
> IR/SafepointIRVerifier.h there.
>
> Since this is a generic issue that can lead to rather unexpected 
> revert of practically any change,
> it would be cool if we had it being explicitly mentioned in docs along 
> with the answer for these
> two questions:
>
>   How to determine which header belongs to which module, and thus 
> which includes are "prohibited"?
>   What is a recommended way of breaking the cyclic dependency?
Having a pointer to docs on syntax and semantics of module.modulemap 
would also be very helpful.

regards,
   Fedor.
>
> If they are already answered in docs then, please, give me a pointer.
> "modules" is rather overloaded term in LLVM, so searching for module 
> does not really help.
>
> regards,
>   Fedor.
>
>> -- adrian
>>
>>> On Mar 29, 2019, at 6:12 AM, Fedor Sergeev <fedor.sergeev at azul.com> 
>>> wrote:
>>>
>>> Adrian, (or anybody else who could help)
>>>
>>> this seemingly innocent port of SafepointIRVerifier caused a build 
>>> failure in "modular" buildbot.
>>> I'm able to reproduce the failure but I have absolutely no clue on 
>>> whats going on.
>>>
>>> Why SafepointIRVerifier.h gets included into compilations of, say, 
>>> IR/BasicBlock.cpp or IR/AsmWriter.cpp?
>>>
>>> Also, SafepointIRVerifier.h was doing pretty much the same as 
>>> Verifier.h does
>>> (i.e. includes PassManager.h to get PassInfoMixin template definition).
>>>
>>> Why Verifier.h does not cause a similar build issue?
>>>
>>> Could you, please, explain/help with this problem?
>>>
>>> regards,
>>>    Fedor.
>>>
>>> On 3/28/19 9:34 PM, Adrian Prantl via llvm-commits wrote:
>>>> Author: adrian
>>>> Date: Thu Mar 28 11:34:34 2019
>>>> New Revision: 357201
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=357201&view=rev
>>>> Log:
>>>> Temporarily revert "SafepointIRVerifier port to new Pass Manager"
>>>> to unbreak the modular bots and its follow-up commit.
>>>>
>>>> This reverts commit https://reviews.llvm.org/D59825
>>>> because it introduced a
>>>>
>>>> fatal error: cyclic dependency in module 'LLVM_intrinsic_gen': 
>>>> LLVM_intrinsic_gen -> LLVM_IR -> LLVM_intrinsic_gen
>>>>
>>>> Modified:
>>>>      llvm/trunk/include/llvm/IR/SafepointIRVerifier.h
>>>>      llvm/trunk/lib/IR/SafepointIRVerifier.cpp
>>>>      llvm/trunk/lib/Passes/PassBuilder.cpp
>>>>      llvm/trunk/lib/Passes/PassRegistry.def
>>>>
>>>> Modified: llvm/trunk/include/llvm/IR/SafepointIRVerifier.h
>>>> URL: 
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/SafepointIRVerifier.h?rev=357201&r1=357200&r2=357201&view=diff
>>>> ============================================================================== 
>>>>
>>>> --- llvm/trunk/include/llvm/IR/SafepointIRVerifier.h (original)
>>>> +++ llvm/trunk/include/llvm/IR/SafepointIRVerifier.h Thu Mar 28 
>>>> 11:34:34 2019
>>>> @@ -18,8 +18,6 @@
>>>>   #ifndef LLVM_IR_SAFEPOINT_IR_VERIFIER
>>>>   #define LLVM_IR_SAFEPOINT_IR_VERIFIER
>>>>   -#include "llvm/IR/PassManager.h"
>>>> -
>>>>   namespace llvm {
>>>>     class Function;
>>>> @@ -31,16 +29,6 @@ void verifySafepointIR(Function &F);
>>>>   /// Create an instance of the safepoint verifier pass which can 
>>>> be added to
>>>>   /// a pass pipeline to check for relocation bugs.
>>>>   FunctionPass *createSafepointIRVerifierPass();
>>>> -
>>>> -/// Create an instance of the safepoint verifier pass which can be 
>>>> added to
>>>> -/// a pass pipeline to check for relocation bugs.
>>>> -class SafepointIRVerifierPass : public 
>>>> PassInfoMixin<SafepointIRVerifierPass> {
>>>> -
>>>> -public:
>>>> -  explicit SafepointIRVerifierPass() {}
>>>> -
>>>> -  PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
>>>> -};
>>>>   }
>>>>     #endif // LLVM_IR_SAFEPOINT_IR_VERIFIER
>>>>
>>>> Modified: llvm/trunk/lib/IR/SafepointIRVerifier.cpp
>>>> URL: 
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/SafepointIRVerifier.cpp?rev=357201&r1=357200&r2=357201&view=diff
>>>> ============================================================================== 
>>>>
>>>> --- llvm/trunk/lib/IR/SafepointIRVerifier.cpp (original)
>>>> +++ llvm/trunk/lib/IR/SafepointIRVerifier.cpp Thu Mar 28 11:34:34 2019
>>>> @@ -197,17 +197,6 @@ protected:
>>>>   static void Verify(const Function &F, const DominatorTree &DT,
>>>>                      const CFGDeadness &CD);
>>>>   -namespace llvm {
>>>> -PreservedAnalyses SafepointIRVerifierPass::run(Function &F,
>>>> - FunctionAnalysisManager &AM) {
>>>> -  const auto &DT = AM.getResult<DominatorTreeAnalysis>(F);
>>>> -  CFGDeadness CD;
>>>> -  CD.processFunction(F, DT);
>>>> -  Verify(F, DT, CD);
>>>> -  return PreservedAnalyses::all();
>>>> -}
>>>> -}
>>>> -
>>>>   namespace {
>>>>     struct SafepointIRVerifier : public FunctionPass {
>>>>
>>>> Modified: llvm/trunk/lib/Passes/PassBuilder.cpp
>>>> URL: 
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Passes/PassBuilder.cpp?rev=357201&r1=357200&r2=357201&view=diff
>>>> ============================================================================== 
>>>>
>>>> --- llvm/trunk/lib/Passes/PassBuilder.cpp (original)
>>>> +++ llvm/trunk/lib/Passes/PassBuilder.cpp Thu Mar 28 11:34:34 2019
>>>> @@ -56,7 +56,6 @@
>>>>   #include "llvm/IR/Dominators.h"
>>>>   #include "llvm/IR/IRPrintingPasses.h"
>>>>   #include "llvm/IR/PassManager.h"
>>>> -#include "llvm/IR/SafepointIRVerifier.h"
>>>>   #include "llvm/IR/Verifier.h"
>>>>   #include "llvm/Support/Debug.h"
>>>>   #include "llvm/Support/FormatVariadic.h"
>>>>
>>>> Modified: llvm/trunk/lib/Passes/PassRegistry.def
>>>> URL: 
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Passes/PassRegistry.def?rev=357201&r1=357200&r2=357201&view=diff
>>>> ============================================================================== 
>>>>
>>>> --- llvm/trunk/lib/Passes/PassRegistry.def (original)
>>>> +++ llvm/trunk/lib/Passes/PassRegistry.def Thu Mar 28 11:34:34 2019
>>>> @@ -231,7 +231,6 @@ FUNCTION_PASS("verify<domtree>", Dominat
>>>>   FUNCTION_PASS("verify<loops>", LoopVerifierPass())
>>>>   FUNCTION_PASS("verify<memoryssa>", MemorySSAVerifierPass())
>>>>   FUNCTION_PASS("verify<regions>", RegionInfoVerifierPass())
>>>> -FUNCTION_PASS("verify<safepoint-ir>", SafepointIRVerifierPass())
>>>>   FUNCTION_PASS("view-cfg", CFGViewerPass())
>>>>   FUNCTION_PASS("view-cfg-only", CFGOnlyViewerPass())
>>>>   FUNCTION_PASS("transform-warning", WarnMissedTransformationsPass())
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>