Page MenuHomePhabricator

[LegacyPassManager] Fix dumpPassArguments() for clang
Needs RevisionPublic

Authored by tyb0807 on Jul 13 2018, 6:29 AM.

Details

Reviewers
dexonsmith
Summary

Hi,

Here's the current situation:

$ opt -debug-pass=Structure -S -print-after-all -O0 < test.ll
Pass Arguments:  -tti -verify -print-function -ee-instrument -print-function
Target Transform Information
  FunctionPass Manager
    Module Verifier
    Print Function IR
    Instrument function entry/exit with calls to e.g. mcount() (pre inlining)
    Print Function IR

whereas:

$ clang -mllvm -debug-pass=Structure -mllvm -print-after-all -emit-llvm -S -O0 -o - test.c
Pass Arguments:  -tti -targetlibinfo -verify -ee-instrument
Target Transform Information
Target Library Information
  FunctionPass Manager
    Module Verifier
    Print Function IR
    Instrument function entry/exit with calls to e.g. mcount() (pre inlining)
    Print Function IR

As you can see, the Pass Arguments display for clang is not accurate: it does not have print-function command, while the PrintFunctionPass is indeed in the pipeline and gets executed (as pointed out by the pipeline structure).

This patch is supposed to fix this: PMDataManager::dumpPassArguments() will consider different IR printing passes (PrintModulePass, PrintFunctionPass and PrintBasicBlockPass). As for PrintCallGraphSCCPass, PrintLoopPass and PrintRegionPass, users can't explicitly insert them into the pipeline (there's no command line argument to do it), so I guess it is correct to not see them in debug-pass=Arguments even though debug-pass=Structure does show that they are in the pipeline.

Sorry to bother you again Duncan , I don't know anybody else to help me review this other than you.

Thank you very much

Diff Detail

Event Timeline

tyb0807 created this revision.Jul 13 2018, 6:29 AM
dexonsmith requested changes to this revision.Oct 16 2020, 7:27 PM

I completely missed this until now :/. I'm happy to review if you're still interested in pursuing this; but glancing at IRPrintingPasses.cpp, this needs to be rebased (=> "request changes"), and I wonder if it's still relevant. If you rebase I can take a look!

This revision now requires changes to proceed.Oct 16 2020, 7:27 PM