Assertion when Module Pass requires Function Pass

[llvmbot]

Bugzilla Link	1958
Resolution	WONTFIX
Resolved on	Apr 17, 2019 16:52
Version	unspecified
OS	Linux
Attachments	Simple module pass requiring LoopInfo
Reporter	LLVM Bugzilla Contributor

Extended Description

The attached pass generates the following assertion:

opt -disable-output -disable-verify -debug-pass=Details -load ./llvm/Debug/lib/TEST.so -TEST test.bc

Pass Arguments: -TEST
Target Data Layout
ModulePass Manager
A TEST PASS
Unnamed pass: implement Pass::getPassName()
-- A TEST PASS
0x86d00e0 Executing Pass 'A TEST PASS' on Module '/home/koy/research/test/testcases/relationships2.bc'...
0x86cd940 Required Analyses: Natural Loop Construction
Pass Arguments: -domtree -loops
FunctionPass Manager
Dominator Tree Construction
Natural Loop Construction

opt: {snip}/llvm/include/llvm/PassAnalysisSupport.h:226: AnalysisType& llvm::Pass::getAnalysisID(const llvm::PassInfo*, llvm::Function&) [with AnalysisType = llvm::LoopInfo]: Assertion `ResultPass && "getAnalysis*() called on an analysis that was not " "'required' by pass!"' failed.

The documentation (http://llvm.org/docs/WritingAnLLVMPass.html#ModulePass) says that this should be possible.

[llvmbot]

This is not supported in current implementation, though I do not see why one could not extend current implementation to support this.

Right now, Module level passes can require other module level passes or on-demand function level passes.

[llvmbot]

This is not supported in current implementation, though I do not see why one
could not extend current implementation to support this.

Right now, Module level passes can require other module level passes or
on-demand function level passes.

The current assertion is not really helpful in this regard. Unfortunately, I'm not familiar with the pass manager system to do any of this development -- either to add a better assertion or to extend the implementation of the PassManager

Of note is a comment I found in the Loop Extractor pass: http://llvm.org/doxygen/LoopExtractor_8cpp-source.html

I do wonder how many others should be method passes, but aren't because of this.

[llvmbot]

Strange. I wrote a module level pass that needed LoopInfo one day and it worked. I have just checked your pass with the svn version of LLVM and I haven't seen the assertion failure. What version are you using?

As Devang wrote, module level passes may currently use function level analysis (such as LoopInfo). I suppose, that even LoopExtractor could be changed to a module pass now. However, there are still problems with more complicated dependendences between passes (i.e. #2291 ).

[llvmbot]

I tried LLVM 2.2 on a different machine with no assertion. I almost closed the bug, but then I tried LLVM 2.2 on the current machine and the assertion still occurred.

Each is running a different OS (it fails on Gentoo but is OK on Ubuntu), so that might be the issue. Or maybe there's a difference in the the tool versions. I don't know where to start.

[llvmbot]

This is interesting...

It turns out I was running the code on different .bc files on either machine. Apply a little bugpoint, and voila!

The problem seems to be that the pass is being run on functions that are only declared, not defined. Without a body, no wonder the pass can't be run.

The workaround is simple ... do a test on isDeclaration first. I'd rather see a more relevant message, though.

[llvmbot]

Right. Now I remember I had a similar issue once - I forgot to add a check whether the function has body before getting analysis results. I think the issue also appeared on the list some time ago...

Unfortunately, I'm not very familiar with PassManager internals.

Devang, do you think it's possible to differentiate these two cases:

• asking for analysis results, but the analysis wasn't requested,
• asking for results of an analysis that was requested, but for a function without body
  and fire different assertions?

[llvmbot]

Does following patch help you ?

Index: PassAnalysisSupport.h

--- PassAnalysisSupport.h (revision 47140)
+++ PassAnalysisSupport.h (working copy)
@@ -20,7 +20,7 @@
#define LLVM_PASS_ANALYSIS_SUPPORT_H

#include

+#include "llvm/Function.h"
namespace llvm {

// No need to include Pass.h, we are being included by it!
@@ -209,7 +209,8 @@
template
AnalysisType &Pass::getAnalysis(Function &F) {
assert(Resolver &&"Pass has not been inserted into a PassManager object!");

• assert(F.isDeclaration() == false &&

•     "Unable to collect analysis info for function declaration");
  

  return getAnalysisID(getClassPassInfo(), F);
  }

[llvmbot]

Thanks, it works for the test pass. I think it may be worthy to commit the patch as forgetting to use isDeclaration() seems to be quite common for people starting to write LLVM passes. It would have surely helped me in the past;)

[lattner]

devang, what is the status of this?

[llvmbot]

This is about adding an assert to catch getAnalysis() requests on a function declaration. The proposed patch requires including Function.h in PassAnalysisSupport.h, which is why I'm hesitant. I am OK, if we close this as won't fix.