Page MenuHomePhabricator

[NewPM] Add an option to dump pass structure
ClosedPublic

Authored by evgeny777 on Mar 30 2021, 8:42 AM.

Details

Summary

This is similar to legacy PM -debug-pass=Structure. It overlaps with -debug-pass-manager, however the latter dumps a lot of stuff which is not related to pass structure (and probably should be printed with LLVM_DEBUG).

Diff Detail

Event Timeline

evgeny777 created this revision.Mar 30 2021, 8:42 AM
evgeny777 requested review of this revision.Mar 30 2021, 8:42 AM
Herald added a project: Restricted Project. · View Herald Transcript
Herald added a subscriber: sstefan1. · View Herald Transcript
evgeny777 updated this revision to Diff 334613.Apr 1 2021, 1:16 AM

Fix windows buildbot failure

tejohnson added inline comments.Fri, Apr 16, 12:15 PM
llvm/lib/Passes/StandardInstrumentations.cpp
56

I noticed that in clang there is an -fdebug-pass-structure that eventually gets translated to the old PM's -debug-pass=Structure. Perhaps that should specify this internal option in the case of the new PM?

907

Can you instead use something like:
dbgs().indent(Ident) << Msg;

evgeny777 updated this revision to Diff 339988.Fri, Apr 23, 5:12 AM

Addressed comments

tejohnson added inline comments.Fri, Apr 23, 10:18 AM
clang/lib/CodeGen/BackendUtil.cpp
872

Looks like there is no existing test for -fdebug-pass-manager, even for the legacy PM. Could you add one? It looks like the way to test this would be via "-mllvm -debug-only=commandline" which currently emits:

Args: clang -debug-pass Structure

llvm/include/llvm/IR/PassManagerImpl.h
77 ↗(On Diff #339988)

What is this check needed for?

evgeny777 added inline comments.Wed, Apr 28, 3:44 AM
llvm/include/llvm/IR/PassManagerImpl.h
77 ↗(On Diff #339988)

For runBeforeAnalysis and runAfterAnalysis to be in sync. See if block above:

if (ID != PassInstrumentationAnalysis::ID()) {
      PI = getResult<PassInstrumentationAnalysis>(IR, ExtraArgs...);
      PI.runBeforeAnalysis(P, IR);
}
evgeny777 updated this revision to Diff 341140.Wed, Apr 28, 4:09 AM

Added clang test

tejohnson added inline comments.Wed, Apr 28, 8:14 AM
llvm/include/llvm/IR/PassManagerImpl.h
77 ↗(On Diff #339988)

Sure I saw that above. But I guess my question is what is the practical effect of this change, i.e. why is it needed with your other changes in this patch when it wasn't needed before?

evgeny777 added inline comments.Wed, Apr 28, 8:23 AM
llvm/include/llvm/IR/PassManagerImpl.h
77 ↗(On Diff #339988)

I felt this is right change, however I see no effect from it, so let's undo it for now

evgeny777 updated this revision to Diff 341219.Wed, Apr 28, 8:24 AM

Addressed comments

This revision is now accepted and ready to land.Wed, Apr 28, 8:55 AM
This revision was landed with ongoing or failed builds.Thu, Apr 29, 12:29 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptThu, Apr 29, 12:29 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
haowei added a subscriber: haowei.Thu, Apr 29, 10:40 AM

We are seeing test failures in our builders with message after your patch. Could you take a look and fix this unit test please?

FAIL: Clang :: Driver/debug-pass-structure.c (6773 of 27894)
******************** TEST 'Clang :: Driver/debug-pass-structure.c' FAILED ********************
Script:
--
: 'RUN: at line 2';   /opt/s/w/ir/x/w/staging/llvm_build/bin/clang -fexperimental-new-pass-manager -fdebug-pass-structure -O3 -S -emit-llvm /opt/s/w/ir/x/w/llvm-project/clang/test/Driver/debug-pass-structure.c -o /dev/null 2>&1 | /opt/s/w/ir/x/w/staging/llvm_build/bin/FileCheck /opt/s/w/ir/x/w/llvm-project/clang/test/Driver/debug-pass-structure.c --check-prefix=NEWPM
: 'RUN: at line 3';   /opt/s/w/ir/x/w/staging/llvm_build/bin/clang -flegacy-pass-manager -fdebug-pass-structure -O0 -S -emit-llvm /opt/s/w/ir/x/w/llvm-project/clang/test/Driver/debug-pass-structure.c -o /dev/null 2>&1 | /opt/s/w/ir/x/w/staging/llvm_build/bin/FileCheck /opt/s/w/ir/x/w/llvm-project/clang/test/Driver/debug-pass-structure.c --check-prefix=LEGACYPM
--
Exit Code: 1

Command Output (stderr):
--
/opt/s/w/ir/x/w/llvm-project/clang/test/Driver/debug-pass-structure.c:46:19: error: LEGACYPM-NEXT: expected string not found in input
// LEGACYPM-NEXT: Pass Arguments: -tti -targetlibinfo -assumption-cache-tracker -profile-summary-info -annotation2metadata -forceattrs -basiccg -always-inline -annotation-remarks
                  ^
<stdin>:5:17: note: scanning from here
 Module Verifier
                ^
<stdin>:6:1: note: possible intended match here
Pass Arguments: -tti -targetlibinfo -assumption-cache-tracker -profile-summary-info -annotation2metadata -forceattrs -basiccg -always-inline -barrier -annotation-remarks
^

Input file: <stdin>
Check file: /opt/s/w/ir/x/w/llvm-project/clang/test/Driver/debug-pass-structure.c

-dump-input=help explains the following input dump.

Input was:
<<<<<<
           1: Pass Arguments: -tti -targetlibinfo -verify 
           2: Target Transform Information 
           3: Target Library Information 
           4:  FunctionPass Manager 
           5:  Module Verifier 
next:46'0                     X error: no match found
           6: Pass Arguments: -tti -targetlibinfo -assumption-cache-tracker -profile-summary-info -annotation2metadata -forceattrs -basiccg -always-inline -barrier -annotation-remarks 
next:46'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
next:46'1     ?                                                                                                                                                                          possible intended match
           7: Target Transform Information 
next:46'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           8: Target Library Information 
next:46'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~
           9: Assumption Cache Tracker 
next:46'0     ~~~~~~~~~~~~~~~~~~~~~~~~~
          10: Profile summary info 
next:46'0     ~~~~~~~~~~~~~~~~~~~~~
          11:  ModulePass Manager 
next:46'0     ~~~~~~~~~~~~~~~~~~~~
           .
           .
           .
>>>>>>

@haowei What are LLVM configuration options? Also please send output from

/opt/s/w/ir/x/w/staging/llvm_build/bin/clang -flegacy-pass-manager -fdebug-pass-structure -O0 -S -emit-llvm /opt/s/w/ir/x/w/llvm-project/clang/test/Driver/debug-pass-structure.c -o /dev/null 2>&1

Hm ... I see BarrierNoop pass being added before annotation-remarks. Why's that?

@haowei What are LLVM configuration options? Also please send output from

/opt/s/w/ir/x/w/staging/llvm_build/bin/clang -flegacy-pass-manager -fdebug-pass-structure -O0 -S -emit-llvm /opt/s/w/ir/x/w/llvm-project/clang/test/Driver/debug-pass-structure.c -o /dev/null 2>&1

Full cmake command line is:

'/local_path/local_path/bin/cmake',
  '-GNinja',
  '-DCMAKE_MAKE_PROGRAM=/local_path/local_path/ninja',
  '-DCMAKE_INSTALL_PREFIX=',
  '-DCMAKE_C_COMPILER=/local_path/local_path/bin/clang',
  '-DCMAKE_CXX_COMPILER=/local_path/local_path/bin/clang++',
  '-DCMAKE_ASM_COMPILER=/local_path/local_path/bin/clang',
  '-DCMAKE_AR=/local_path/local_path/bin/llvm-ar',
  '-DCMAKE_LINKER=/local_path/local_path/bin/ld.lld',
  '-DCMAKE_NM=/local_path/local_path/bin/llvm-nm',
  '-DCMAKE_OBJCOPY=/local_path/local_path/bin/llvm-objcopy',
  '-DCMAKE_OBJDUMP=/local_path/local_path/bin/llvm-objdump',
  '-DCMAKE_RANLIB=/local_path/local_path/bin/llvm-ranlib',
  '-DCMAKE_READELF=/local_path/local_path/bin/llvm-readelf',
  '-DCMAKE_STRIP=/local_path/local_path/bin/llvm-strip',
  '-DCMAKE_SYSROOT=/local_path/local_path/linux',
  '-DZLIB_INCLUDE_DIR=/local_path/staging/zlib_install/include',
  '-DZLIB_LIBRARY=/local_path/staging/zlib_install/lib/libz.a',
  '-DLIBXML2_INCLUDE_DIR=/local_path/staging/libxml2_install/include/libxml2',
  '-DLIBXML2_LIBRARY=/local_path/staging/libxml2_install/lib/libxml2.a',
  '-DCMAKE_SHARED_LINKER_FLAGS=-static-libstdc++',
  '-DCMAKE_MODULE_LINKER_FLAGS=-static-libstdc++',
  '-DCMAKE_EXE_LINKER_FLAGS=-static-libstdc++',
  '-DLLVM_ENABLE_LTO=False',
  '-DLLVM_ENABLE_ASSERTIONS=True',
  '-DLINUX_aarch64-unknown-linux-gnu_SYSROOT=/local_path/local_path/linux',
  '-DLINUX_armv7-unknown-linux-gnueabihf_SYSROOT=/local_path/local_path/linux',
  '-DLINUX_i386-unknown-linux-gnu_SYSROOT=/local_path/local_path/linux',
  '-DLINUX_x86_64-unknown-linux-gnu_SYSROOT=/local_path/local_path/linux',
  '-DFUCHSIA_SDK=/local_path/local_path/sdk',
  '-C',
  '/local_path/llvm-project/clang/cmake/caches/Fuchsia-stage2.cmake',
  '/local_path/llvm-project/llvm',

I am currently doing a local build to see if I can generate the output you required. We suspect it may be related to ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER which is enabled in clang/cmake/caches/Fuchsia-stage2.cmake we used.

Output from clang -flegacy-pass-manager -fdebug-pass-structure -O0 -S -emit-llvm /opt/s/w/ir/x/w/llvm-project/clang/test/Driver/debug-pass-structure.c -o /dev/null 2>&1:

Pass Arguments:  -tti -targetlibinfo
Target Transform Information
Target Library Information
  FunctionPass Manager
Pass Arguments:  -tti -targetlibinfo -assumption-cache-tracker -profile-summary-info -annotation2metadata -forceattrs -basiccg -always-inline -barrier -annotation-remarks
Target Transform Information
Target Library Information
Assumption Cache Tracker
Profile summary info
  ModulePass Manager
    Annotation2Metadata
    Force set function attributes
    CallGraph Construction
    Call Graph SCC Pass Manager
      Inliner for always_inline functions
    A No-Op Barrier Pass
    FunctionPass Manager
      Annotation Remarks
    Print Module IR
Pass Arguments:  -tti
Target Transform Information
  ModulePass Manager
phosek added a subscriber: phosek.Thu, Apr 29, 9:59 PM

It looks like the BarrierNoop pass is inserted under -O0 when any extension is registered. We enable Polly in our toolchain so that's likely the reason why we're seeing this on our bots.

Should be fixed with c81ec19fba27ec

Should be fixed with c81ec19fba27ec

Thank you, our builders went green after that change.

As you said in the description, this overlaps a lot with -debug-pass-manager. What exactly is too verbose with the existing debug logging? I'd strongly prefer to have this consolidated into one pass instrumentation rather than two.

Having a new pipeline test is very tedious when changing the pipeline. We already have a bunch.

llvm/lib/Passes/StandardInstrumentations.cpp
23

is this needed?

55

The new PM design tries to avoid global options. We should implement the above FIXME and fold this into PrintPassInstrumentation.

I've already run into having to update these two golden file tests twice, we really shouldn't be having multiple types of golden file tests for the same thing in different locations and with different formats. I'd like to revert this change.

Perhaps something like https://reviews.llvm.org/D101797 and maybe some more followups are better.

I've already run into having to update these two golden file tests twice

I think you can just reduce the tests. No need to revert the entire change, unless there is a replacement for it (D101797 is not)

I've already run into having to update these two golden file tests twice

I think you can just reduce the tests. No need to revert the entire change, unless there is a replacement for it (D101797 is not)

Could you explain why -debug-pass-manager doesn't fit your use case?

Could you explain why -debug-pass-manager doesn't fit your use case?

I wanted to have something similar to -debug-pass=Structure, because the above is too verbose and lacks identation. What's the problem with this one, anyway?