Page MenuHomePhabricator

Introduce -print-before-changed, making -print-changed also print before passes that modify IR
ClosedPublic

Authored by jamieschmeiser on Oct 2 2020, 3:01 PM.

Details

Summary

Add an option -print-before-changed that modifies the print-changed
behaviour so that it prints the IR before a pass that changed it in
addition to printing the IR after the pass. Note that the option
does nothing in isolation. The filtering options work as expected.
Lit tests are included.

Diff Detail

Event Timeline

jamieschmeiser created this revision.Oct 2 2020, 3:01 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 2 2020, 3:01 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
jamieschmeiser requested review of this revision.Oct 2 2020, 3:01 PM
aeubanks accepted this revision.Oct 22 2020, 8:57 AM

Could you capitalize and shorten the title a bit? Something like "Introduce -print-before-changed, making -print-changed also print before passes that modify IR"

This revision is now accepted and ready to land.Oct 22 2020, 8:57 AM
jamieschmeiser updated this revision to Diff 300019.EditedOct 22 2020, 9:42 AM

Changed name of option to -also-print-before to avoid common option prefix problem.

jamieschmeiser retitled this revision from add new option print-before-changed that modifies print-changed to also print before passes that change the IR to Introduce -also-print-before, making -print-changed also print before passes that modify IR.Oct 22 2020, 9:47 AM
jamieschmeiser edited the summary of this revision. (Show Details)

@aeubanks, just want to point out that I have changed the option name to avoid the problem of -print-before and -print-before-changed having a common prefix. The option name is now -also-print-before. Please verify that this is acceptable and then I will deliver this.

I don't like -also-print-before, it's too generic. I'd prefer something like -print-before-changed.

Changed name of option to -also-print-before to avoid common option prefix problem.

Please fix the revision subject as well.

@aeubanks, I agree but when I first put up this PR, there was a lit test failure and looking at the run command for the test, it had "/fuchsia.cpp -### -no-canonical-prefixes" in it, so I think the problem is that the existing -print-before is a prefix of -print-before-changed. How about -also-print-before-changed?

@aeubanks, I agree but when I first put up this PR, there was a lit test failure and looking at the run command for the test, it had "/fuchsia.cpp -### -no-canonical-prefixes" in it, so I think the problem is that the existing -print-before is a prefix of -print-before-changed. How about -also-print-before-changed?

I thought -### added quotes to not have this sort of issue.
What exactly is the test failure?

@aeubanks, you can see the failure report by looking at Diff 1 in the history tab of the revision contents in this PR (https://reviews.llvm.org/D88757?id=295916):

linux > Clang.Driver::fuchsia.cpp

Script:

: 'RUN: at line 1'; /mnt/disks/ssd0/agent/llvm-project/build/bin/clang --driver-mode=g++ /mnt/disks/ssd0/agent/llvm-project/clang/test/Driver/fuchsia.cpp -### -no-canonical-prefixes --target=x86_64-fuchsia -ccc-install-dir /mnt/disks/ssd0/agent/llvm-project/clang/test/Driver/Inputs/basic_fuchsia_tree/bin -resource-dir=/mnt/disks/ssd0/agent/llvm-project/clang/test/Driver/Inputs/resource_dir_with_per_target_subdir --sysroot=/mnt/disks/ssd0/agent/llvm-project/clang/test/Driver/platform -fuse-ld=lld 2>&1 | /mnt/disks/ssd0/agent/llvm-project/build/bin/FileCheck -check-prefixes=CHECK,CHECK-X86_64 /mnt/disks/ssd0/agent/llvm-project/clang/test/Driver/fuchsia.cpp

That failure doesn't look related to this change, looks like something else broke it. Can you repro locally?

jamieschmeiser retitled this revision from Introduce -also-print-before, making -print-changed also print before passes that modify IR to Introduce -print-before-changed, making -print-changed also print before passes that modify IR.
jamieschmeiser edited the summary of this revision. (Show Details)

@aeubanks, it appears that you are correct that the previous error was not caused by my changes as I cannot reproduce it locally. I have changed the option back to -print-before-changed.