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).
Details
Diff Detail
Unit Tests
Time | Test | |
---|---|---|
2,600 ms | x64 debian > libarcher.races::lock-unrelated.c |
Event Timeline
llvm/lib/Passes/StandardInstrumentations.cpp | ||
---|---|---|
55 | 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? | |
923 | Can you instead use something like: |
clang/lib/CodeGen/BackendUtil.cpp | ||
---|---|---|
872 ↗ | (On Diff #339988) | 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–78 | What is this check needed for? |
llvm/include/llvm/IR/PassManagerImpl.h | ||
---|---|---|
77–78 | 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); } |
llvm/include/llvm/IR/PassManagerImpl.h | ||
---|---|---|
77–78 | 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? |
llvm/include/llvm/IR/PassManagerImpl.h | ||
---|---|---|
77–78 | I felt this is right change, however I see no effect from it, so let's undo it for now |
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
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
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.
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? | |
54 | 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)
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?
What is this check needed for?