Page MenuHomePhabricator

[NewPM] Support --print-before/after in NPM
Needs ReviewPublic

Authored by aeubanks on Sep 6 2020, 6:38 PM.

Details

Summary
This changes --print-before/after to be a list of strings rather than
legacy passes. (this also has the effect of not showing the entire list
of passes in --help-hidden after --print-before/after, which IMO is
great for making it less verbose).

Currently PrintIRInstrumentation passes the class name rather than pass
name to llvm::shouldPrintBeforePass(), meaning
llvm::shouldPrintBeforePass() never functions as intended in the NPM.
There is no easy way of converting class names to pass names outside of
within an instance of PassBuilder.

This adds a map of pass class names to their short names in
PassRegistry.def within PassInstrumentationCallbacks. It is populated
inside the constructor of PassBuilder, which takes a
PassInstrumentationCallbacks.

Add a pointer to PassInstrumentationCallbacks inside
PrintIRInstrumentation and use the newly created map.

This is a bit hacky, but I can't think of a better way since the short
id to class name only exists within PassRegistry.def. This also doesn't
handle passes not in PassRegistry.def but rather added via
PassBuilder::registerPipelineParsingCallback().

llvm/test/CodeGen/Generic/print-after.ll doesn't seem very useful now
with this change.

Diff Detail

Unit TestsFailed

TimeTest
420 mslinux > HWAddressSanitizer-x86_64.TestCases::sizes.cpp
Script: -- : 'RUN: at line 3'; /mnt/disks/ssd0/agent/llvm-project/build/./bin/clang --driver-mode=g++ -m64 -gline-tables-only -fsanitize=hwaddress -fuse-ld=lld -mcmodel=large -mllvm -hwasan-globals -mllvm -hwasan-use-short-granules -mllvm -hwasan-instrument-landing-pads=0 -mllvm -hwasan-instrument-personality-functions /mnt/disks/ssd0/agent/llvm-project/compiler-rt/test/hwasan/TestCases/sizes.cpp -nostdlib++ -lstdc++ -o /mnt/disks/ssd0/agent/llvm-project/build/projects/compiler-rt/test/hwasan/X86_64/TestCases/Output/sizes.cpp.tmp

Event Timeline

aeubanks created this revision.Sep 6 2020, 6:38 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptSep 6 2020, 6:38 PM
aeubanks requested review of this revision.Sep 6 2020, 6:38 PM
aeubanks updated this revision to Diff 290159.Sep 6 2020, 6:40 PM

remove some includes

aeubanks updated this revision to Diff 300555.Oct 25 2020, 1:40 PM

move map into PassInstrumentationCallbacks

aeubanks edited the summary of this revision. (Show Details)Oct 25 2020, 1:42 PM
aeubanks added a reviewer: asbirlea.
jamieschmeiser requested changes to this revision.Oct 27 2020, 9:25 AM

The changes are specific to -print-before and -print-after (which is the intended target and this work originated before -print-changed) but could the change be made universal? I would rather see the existing StringRef passed to all the callbacks changed rather than adding the new StringRef to specific callbacks. This way all passes benefit and are consistent. In particular, I would like to see this change applied to -printChanged; this came up in the reviews for that PR. If you would rather not make it universal, then at least change all of the callbacks to also receive the new StringRef.

This revision now requires changes to proceed.Oct 27 2020, 9:25 AM
aeubanks updated this revision to Diff 306307.Wed, Nov 18, 9:50 PM

rebase
now we don't add a new parameter to callbacks, instead let individual instrumentations ask PassInstrumentationCallbacks for pass names

aeubanks edited the summary of this revision. (Show Details)Wed, Nov 18, 9:50 PM

The changes are specific to -print-before and -print-after (which is the intended target and this work originated before -print-changed) but could the change be made universal? I would rather see the existing StringRef passed to all the callbacks changed rather than adding the new StringRef to specific callbacks. This way all passes benefit and are consistent. In particular, I would like to see this change applied to -printChanged; this came up in the reviews for that PR. If you would rather not make it universal, then at least change all of the callbacks to also receive the new StringRef.

I think this change might be a better way, PTAL