This is an archive of the discontinued LLVM Phabricator instance.

Differential D147394

[NewPM] Use PassID instead of pass name
ClosedPublic

Authored by pchintalapudi on Apr 1 2023, 4:51 PM.

Details

Reviewers
aeubanks
Commits
rGd4de7c2e1e79: [NewPM] Use PassID instead of pass name
Summary

PrintIRInstrumentation::shouldPrintAfterPass accepts a pass ID instead of a pass name

Diff Detail

Unit TestsFailed

TimeTest
70 msx64 debian > lld.wasm::stub-library.s

Event Timeline

pchintalapudi created this revision.Apr 1 2023, 4:51 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 1 2023, 4:51 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
pchintalapudi requested review of this revision.Apr 1 2023, 4:51 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 1 2023, 4:51 PM
Herald added a subscriber: llvm-commits. · View Herald Transcript
Harbormaster completed remote builds in B223195: Diff 510278.Apr 1 2023, 5:40 PM
aeubanks added a comment.Apr 2 2023, 12:00 AM

lg, but can you add a test?

pchintalapudi added a comment.Apr 3 2023, 1:17 PM

How do I go about adding a regression test for this? I have a simple reproducer here: https://godbolt.org/z/nEK139rEo

aeubanks added a comment.Apr 13 2023, 10:55 AM

something along the lines of llvm/test/Other/loop-pass-printer.ll with the IR and opt command you have should be good

pchintalapudi updated this revision to Diff 513346.Apr 13 2023, 1:59 PM

Add test

Harbormaster completed remote builds in B225432: Diff 513346.Apr 13 2023, 2:42 PM
aeubanks added a comment.Apr 13 2023, 3:07 PM

some test nits, otherwise lgtm
do you need me to land this for you?

llvm/test/Other/loop-print-after-pass-invalidated.ll
3
5

there's no DEL check prefix in the test?

aeubanks added a comment.Apr 13 2023, 3:07 PM

also the added test fails according to the premerge checks

pchintalapudi updated this revision to Diff 513396.Apr 13 2023, 5:33 PM

Fix test

Harbormaster completed remote builds in B225465: Diff 513396.Apr 13 2023, 6:03 PM
aeubanks accepted this revision.Apr 14 2023, 9:59 AM

lg with one more test fix

llvm/test/Other/loop-print-after-pass-invalidated.ll
3

this suggested fix is still relevant

This revision is now accepted and ready to land.Apr 14 2023, 9:59 AM
pchintalapudi updated this revision to Diff 513699.Apr 14 2023, 11:50 AM

Simplify opt arguments

pchintalapudi added a comment.Apr 14 2023, 11:52 AM

do you need me to land this for you?

Yes please,

Name: Prem Chintalapudi
Email: prem.chintalapudi@gmail.com

Harbormaster completed remote builds in B225690: Diff 513699.Apr 14 2023, 2:03 PM
This revision was landed with ongoing or failed builds.Apr 14 2023, 5:58 PM
Closed by commit rGd4de7c2e1e79: [NewPM] Use PassID instead of pass name (authored by pchintalapudi, committed by aeubanks). · Explain Why
This revision was automatically updated to reflect the committed changes.
aeubanks added a commit: rGd4de7c2e1e79: [NewPM] Use PassID instead of pass name.

Revision Contents

PathSize
llvm/
lib/
Passes/
3 lines
test/
Other/
21 lines

Diff 513699

llvm/lib/Passes/StandardInstrumentations.cpp

Show First 20 Lines • Show All 720 Lines • ▼ Show 20 Linesvoid PrintIRInstrumentation::printAfterPass(StringRef PassID, Any IR) {
if (!shouldPrintIR(IR)) if (!shouldPrintIR(IR))
return; return;
dbgs() << "*** IR Dump After " << PassID << " on " << IRName << " ***\n"; dbgs() << "*** IR Dump After " << PassID << " on " << IRName << " ***\n";
unwrapAndPrint(dbgs(), IR); unwrapAndPrint(dbgs(), IR);
} }
void PrintIRInstrumentation::printAfterPassInvalidated(StringRef PassID) { void PrintIRInstrumentation::printAfterPassInvalidated(StringRef PassID) {
StringRef PassName = PIC->getPassNameForClassName(PassID); if (!shouldPrintAfterPass(PassID))
if (!shouldPrintAfterPass(PassName))
return; return;
if (isIgnored(PassID)) if (isIgnored(PassID))
return; return;
const Module *M; const Module *M;
std::string IRName; std::string IRName;
StringRef StoredPassID; StringRef StoredPassID;
▲ Show 20 Lines • Show All 1,532 Lines • Show Last 20 Lines

llvm/test/Other/loop-print-after-pass-invalidated.ll

  • This file was added.
; RUN: opt < %s 2>&1 -disable-output \
; RUN: -passes='simple-loop-unswitch<nontrivial>' \
; RUN: -print-after=simple-loop-unswitch \
aeubanksUnsubmitted
Not Done
ReplyInline Actions
; RUN: opt < %s 2>&1 -disable-output \
- ; RUN: -passes='loop(simple-loop-unswitch)' -enable-nontrivial-unswitch \
+ ; RUN: -passes='simple-loop-unswitch<nontrivial>' \
; RUN: -print-after=simple-loop-unswitch \
aeubanks:
aeubanksUnsubmitted
Not Done
ReplyInline Actions

this suggested fix is still relevant

aeubanks: this suggested fix is still relevant
; RUN: | FileCheck %s
aeubanksUnsubmitted
Not Done
ReplyInline Actions

there's no DEL check prefix in the test?

aeubanks: there's no `DEL` check prefix in the test?
; CHECK: *** IR Dump After SimpleLoopUnswitchPass on for.cond ***
; CHECK: *** IR Dump After SimpleLoopUnswitchPass on for.cond.us ***
define void @loop(i1 %w) {
entry:
br label %for.cond
; Loop:
for.cond: ; preds = %for.inc, %entry
br i1 %w, label %for.inc, label %if.then
if.then: ; preds = %for.cond
br label %for.inc
for.inc: ; preds = %if.then, %for.cond
br label %for.cond
}