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

Sat Mar 30 07:46:23 PDT 2019

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?

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