This is an archive of the discontinued LLVM Phabricator instance.

[Utils] Added the ability to print the pass number and IR after it is triggered
ClosedPublic

Authored by dbakunevich on Apr 27 2023, 6:59 AM.

Details

Summary
As part of this patch, 2 options have been added:
print-pass-numbers and print-after-pass-number.

1) The print-pass-numbers option allows to print the pass names and their ordinals.
   The output of the option looks like this:
                Running pass ORDINAL PASS_NAME

2) The print-after-pass-number option allows to print IR after pass with the number
   which reported by print-passes-names.

Diff Detail

Event Timeline

dbakunevich created this revision.Apr 27 2023, 6:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 27 2023, 6:59 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
dbakunevich requested review of this revision.Apr 27 2023, 6:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 27 2023, 6:59 AM

just curious, what's the use case for this?

llvm/lib/Passes/StandardInstrumentations.cpp
749

seems like this should be part of PrintPassInstrumentation instead

752

this duplicates the logic below and always prints the entire module, can you keep it more consistent with the current code?

llvm/test/Other/print-after-pass-number.ll
1 ↗(On Diff #517545)

this shouldn't use update_test_checks.py, it should be checking the headers the instrumentation prints

llvm/test/Other/print-pass-number.ll
3 ↗(On Diff #517545)

I don't think this requires asserts

We would like to have a way to take a snapshot of the IR at a specific point in the pipeline. Currently, you can either print the IR before/after a given pass (-print-before=<passname>) or print before/after all (-print-before-all). print-before will print the IR before all occurrences of the given pass. print-before-all will, well, print before all passes. While print-before-all can be used to achieve the same functionality of printing the pipeline and looking at the IR at a specific point in the pipeline, it's not very practical. For large compilations, print-before-all might take minutes to produce gigabytes of output.

More specifically, we are building a UI tool to inspect the optimization pipeline and would like to use the new command line options for this automation.

dbakunevich added inline comments.May 3 2023, 9:45 AM
llvm/lib/Passes/StandardInstrumentations.cpp
749

I don't agree with you. All the logic described for saving the pass number and subsequent printing of the IR is tied to the logic of registering callbacks in the PrintIRInstrumentation class.

752

Okay, I'll change this moment.

llvm/test/Other/print-after-pass-number.ll
1 ↗(On Diff #517545)

Okay, I'll change this moment.

llvm/test/Other/print-pass-number.ll
3 ↗(On Diff #517545)

Okay, I'll change this moment.

dbakunevich updated this revision to Diff 519220.
dbakunevich marked 3 inline comments as done.
aeubanks added inline comments.May 10 2023, 11:24 AM
llvm/lib/Passes/StandardInstrumentations.cpp
114

so that we don't have an explosion of flag combinations, can we make it so this flag only takes effect when the other -print flags are on? i.e. this doesn't turn on printing on its own, rather it controls what gets printed if we are printing. this does require also passing whatever other flag (e.g. -print-after-all but I think that's fine. this should simplify a lot of the conditionals

119

this seems more like print-at-pass-number rather than print-after-pass-number IIUC

746

might as well do this unconditionally

771–777

put this in an else

802

isn't 0 a valid value? maybe make -1 the sentinel value

dbakunevich marked 4 inline comments as done.May 11 2023, 3:21 AM
dbakunevich added inline comments.
llvm/lib/Passes/StandardInstrumentations.cpp
802

PrintPassNumbers is a bool variable that is either true or false. Perhaps you meant PrintAfterPassNumber. If so, then inside this tool it was customary to count passes starting from 1. This is more convenient from the point of view of human perception. Therefore, the value 0 is not valid.

dbakunevich marked 2 inline comments as done.May 11 2023, 5:01 AM
dbakunevich added inline comments.
llvm/lib/Passes/StandardInstrumentations.cpp
114

I don't agree with you. As part of a particular task, it may be necessary to display only a list of passes with them ordinals without IR.

dbakunevich marked an inline comment as done.
dbakunevich added a reviewer: aeubanks.

mostly looks good

I do think this number of conditions in PrintIRInstrumentation is getting to be a bit much, but for now this is fine

if we're sometimes not actually printing IR in PrintIRInstrumentation with print-pass-numbers and only printing Running pass..., I think we should consider merging it with PrintPassInstrumentation, but that doesn't need to be done here

llvm/lib/Passes/StandardInstrumentations.cpp
119

other function/var names/comments should be updated as well

774

extra space?

llvm/test/Other/print-after-pass-number.ll
5 ↗(On Diff #521261)

we shouldn't check the exact IR here, something like

; AFTER: IR Dump after FooPass-3 on bar
; AFTER-NEXT: define i32 @bar

is better. and also a similar test for one where the IR is invalidated

dbakunevich marked 3 inline comments as done.May 19 2023, 7:30 AM
dbakunevich updated this revision to Diff 523768.
aeubanks added inline comments.May 19 2023, 10:15 AM
llvm/lib/Passes/StandardInstrumentations.cpp
774

ditto for the one in printAfterPass, but actually shouldn't this go in printBeforePass?

llvm/test/Other/print-at-pass-number.ll
9–10

again, we shouldn't be checking the exact IR, that's prone to changes in the passes

and there should be a test for invalidated IR

dbakunevich marked an inline comment as done.May 23 2023, 2:59 AM
dbakunevich added inline comments.
llvm/lib/Passes/StandardInstrumentations.cpp
774

No, this logic should be near with PrintAtPassNumber.

dbakunevich marked 2 inline comments as done.

ping!

lg with the one comment addressed

llvm/lib/Passes/StandardInstrumentations.cpp
774

I definitely think

++CurrentPassNumber;

if (shouldPrintPassNumbers())
  dbgs() << "Running pass " << CurrentPassNumber << " " << PassID << "\n";

should be in printBeforePass, there's no reason to duplicate it between the two printAfterPasses (e.g. the extra space wasn't updated in the copy above)

dbakunevich added inline comments.May 23 2023, 3:45 PM
llvm/lib/Passes/StandardInstrumentations.cpp
774

We can't do that because the before and after passes are different. Accordingly, if we transfer the printing of a pass with its ordinals to printBeforePass(), then the pass numbers will not correspond to those that can be displayed in printAfterPasses().

aeubanks added inline comments.May 24 2023, 11:16 AM
llvm/lib/Passes/StandardInstrumentations.cpp
774

within a pass run, ++CurrentPassNumber happens before any usage of it right now, that won't change by moving it to printBeforePass

llvm/test/Other/print-at-pass-number.ll
9–10

ping on the invalidated IR test

dbakunevich marked 3 inline comments as done.
This revision is now accepted and ready to land.May 25 2023, 2:49 PM
This revision was landed with ongoing or failed builds.May 26 2023, 3:36 AM
This revision was automatically updated to reflect the committed changes.