Show files produced by --emit foo in json artifact notifications

[pacak]

Right now it is possible to ask rustc to save some intermediate representation into one or more files with --emit=foo, but figuring out what exactly was produced is difficult. This pull request adds information about llvm_ir and asm intermediate files into notifications produced by --json=artifacts.

Related discussion: https://internals.rust-lang.org/t/easier-access-to-files-generated-by-emit-foo/20477

Motivation - cargo-show-asm parses those intermediate files and presents them in a user friendly way, but right now I have to apply some dirty hacks. Hacks make behavior confusing: hintron/computer-enhance#35

This pull request introduces a new behavior: now rustc will emit a new artifact notification for every artifact type user asked to --emit, for example for --emit asm those will include all the .s files.

Most users won't notice this behavior, to be affected by it all of the following must hold:

• user must use rustc binary directly (when cargo invokes rustc - it consumes artifact notifications and doesn't emit anything)
• user must specify both --emit xxx and --json artifacts
• user must refuse to handle unknown artifact types
• user must disable incremental compilation (or deal with it better than cargo does, or use a workaround like save-temps) in order not to hit --emit=llvm-bc in workspaces #88829 / Failure on subsequent builds when --emit asm used without -C save-temps. #89149

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @petrochenkov (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

[rustbot]

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

[petrochenkov]

Seems reasonable.
r? @bjorn3 do you have an opinion on this?

[rustbot]

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

[pacak]

I changed the documentation a bit, now it mentions that notifications can have file of the same type mentioned several times, added support for all the files that can be emitted (I hope), changed llvm_bc -> llvm-bc to match the documentation.

Now I'm getting something like this:

% /home/pacak/ej/rust/build/x86_64-unknown-linux-gnu/stage1/bin/rustc --crate-name smo --edition=2021 src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --diagnostic-width=239 --crate-type lib --emit=dep-info,metadata,link,llvm-ir,llvm-bc,asm,mir,obj -C opt-level=3 -C codegen-units=16
{"$message_type":"artifact","artifact":"smo.d","emit":"dep-info"}
{"$message_type":"artifact","artifact":"libsmo.rmeta","emit":"metadata"}
{"$message_type":"artifact","artifact":"smo.mir","emit":"mir"}
{"$message_type":"artifact","artifact":"smo.smo.9eb4b431c6ed4cc9-cgu.2.rcgu.bc","emit":"llvm-bc"}
{"$message_type":"artifact","artifact":"smo.smo.9eb4b431c6ed4cc9-cgu.2.rcgu.ll","emit":"llvm-ir"}
{"$message_type":"artifact","artifact":"smo.smo.9eb4b431c6ed4cc9-cgu.0.rcgu.bc","emit":"llvm-bc"}
{"$message_type":"artifact","artifact":"smo.smo.9eb4b431c6ed4cc9-cgu.0.rcgu.ll","emit":"llvm-ir"}
{"$message_type":"artifact","artifact":"smo.smo.9eb4b431c6ed4cc9-cgu.2.rcgu.s","emit":"asm"}
{"$message_type":"artifact","artifact":"smo.smo.9eb4b431c6ed4cc9-cgu.2.rcgu.o","emit":"obj"}
{"$message_type":"artifact","artifact":"smo.smo.9eb4b431c6ed4cc9-cgu.1.rcgu.bc","emit":"llvm-bc"}
{"$message_type":"artifact","artifact":"smo.smo.9eb4b431c6ed4cc9-cgu.1.rcgu.ll","emit":"llvm-ir"}
{"$message_type":"artifact","artifact":"smo.smo.9eb4b431c6ed4cc9-cgu.0.rcgu.s","emit":"asm"}
{"$message_type":"artifact","artifact":"smo.smo.9eb4b431c6ed4cc9-cgu.0.rcgu.o","emit":"obj"}
{"$message_type":"artifact","artifact":"smo.smo.9eb4b431c6ed4cc9-cgu.1.rcgu.s","emit":"asm"}
{"$message_type":"artifact","artifact":"smo.smo.9eb4b431c6ed4cc9-cgu.1.rcgu.o","emit":"obj"}
{"$message_type":"diagnostic","message":"ignoring emit path because multiple .bc files were produced","code":null,"level":"warning","spans":[],"children":[],"rendered":"\u001b[0m\u001b[1m\u001b[33mwarning\u001b[0m\u001b[0m\u001b[1m: ignoring emit path because multiple .bc files were produced\u001b[0m\n\n"}
{"$message_type":"diagnostic","message":"ignoring emit path because multiple .s files were produced","code":null,"level":"warning","spans":[],"children":[],"rendered":"\u001b[0m\u001b[1m\u001b[33mwarning\u001b[0m\u001b[0m\u001b[1m: ignoring emit path because multiple .s files were produced\u001b[0m\n\n"}
{"$message_type":"diagnostic","message":"ignoring emit path because multiple .ll files were produced","code":null,"level":"warning","spans":[],"children":[],"rendered":"\u001b[0m\u001b[1m\u001b[33mwarning\u001b[0m\u001b[0m\u001b[1m: ignoring emit path because multiple .ll files were produced\u001b[0m\n\n"}
{"$message_type":"diagnostic","message":"ignoring emit path because multiple .o files were produced","code":null,"level":"warning","spans":[],"children":[],"rendered":"\u001b[0m\u001b[1m\u001b[33mwarning\u001b[0m\u001b[0m\u001b[1m: ignoring emit path because multiple .o files were produced\u001b[0m\n\n"}
{"$message_type":"artifact","artifact":"libsmo.rlib","emit":"link"}
{"$message_type":"diagnostic","message":"4 warnings emitted","code":null,"level":"warning","spans":[],"children":[],"rendered":"\u001b[0m\u001b[1m\u001b[33mwarning\u001b[0m\u001b[0m\u001b[1m: 4 warnings emitted\u001b[0m\n\n"}

Error messages are wrong - I'm not passing an emit path, ticket is here #122509 - I think I'll just make a fix later.

Now I'm trying to figure out how to run parser from cargo-metadata on this output...

[pacak]

Well, failing tests are asking to emit obj files, directly or indirectly as well as json information about artifacts. test runner finds unexpected new information about the obj files and fails. This new information is kind of the whole point of this pull request so saving this as expected output makes sense, but I'm not sure about how stable random looking parts of the name are - I don't want to introduce flaky tests.

I think my options are

• don't emit information about obj files unless --emit obj is present
• figure out how to normalize random looking part of the name and save new expected test output...

Went with the normalization approach.

[bjorn3]

Would you mind also adding this to rustc_codegen_cranelift?

The changes in this PR LGTM minus the couple of small changes below. I don't think I can approve an insta-stable change like myself this though. Maybe put it behind an unstable flag?

[pacak]

Would you mind also adding this to rustc_codegen_cranelift?

Is it actually producing intermediate artifacts? I didn't add it because I couldn't find any apart from crate::pretty_clif::write_clif_file which feels like it's doing something slightly different. I can add support for it or any other places if you point them to me,

The changes in this PR LGTM minus the couple of small changes below.

fixed

I don't think I can approve an insta-stable change like myself this though. Maybe put it behind an unstable flag?

Hmm... Should we poke somebody else then? It's not really a behavior change, more like extending existing diagnostic that you have to ask specifically for with two separate flags - emit intermediates and emit artifact notifications.

[bjorn3]

Is it actually producing intermediate artifacts?

It produces object files. Bitcode doesn't exist for Cranelift (and LTO isn't supported either) --emit asm isn't supported either. --emit llvm-ir emits cranelift ir text files, but unlike the other backends it doesn't emit one file per cgu, but a directory with one file for each individual function. It makes sense to me to skip that one too. So Only for object files the artifact notification should be added.

[bjorn3]

Hmm... Should we poke somebody else then?

You could ask on zulip in #t-compiler. Or maybe ask during a compiler team meeting: https://rust-lang.zulipchat.com/#narrow/stream/238009-t-compiler.2Fmeetings (every thursday, not sure about what time it would be in your timezone)

[pacak]

Would you mind also adding this to rustc_codegen_cranelift?

I'm still not sure where it produces those object files or how to compile and run rustc with cranelift. If you can point me at exact line/function - I'll try to add the support, otherwise - I'll pass.

[bjorn3]

The object files for a cgu are all emitted right before

rust/compiler/rustc_codegen_cranelift/src/driver/aot.rs

Line 175 in fa374a8

})

It is probably easier to iterate through modules at

rust/compiler/rustc_codegen_cranelift/src/driver/aot.rs

Line 108 in fa374a8

though as you have access to the Session there and object files are copied from the incr comp cache just a bit earlier in this function. By the way are you emitting artifact notifications for object files copied from the incr comp cache for the LLVM backend?

[rustbot]

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

[pacak]

The object files for a cgu are all emitted right before

Okay, at least I was looking in the right file :) Added, seem to work.

By the way are you emitting artifact notifications for object files copied from the incr comp cache for the LLVM backend?

Added this as well. Now I'm wondering how notifications for asm/llvm are interacting with incremental cache...

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[bjorn3]

@bors r+

[bors]

📌 Commit c8390cd has been approved by bjorn3

It is now in the queue for this repository.

[bors]

⌛ Testing commit c8390cd with merge 1689a5a...

[bors]

☀️ Test successful - checks-actions
Approved by: bjorn3
Pushing 1689a5a to master...

[rust-timer]

Finished benchmarking commit (1689a5a): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.9%	[-0.9%, -0.9%]	1
Improvements ✅ (secondary)	-1.2%	[-1.2%, -1.2%]	1
All ❌✅ (primary)	-0.9%	[-0.9%, -0.9%]	1

Max RSS (memory usage)

Results (secondary 3.2%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.2%	[2.9%, 3.5%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 671.059s -> 673.132s (0.31%)
Artifact size: 319.05 MiB -> 318.94 MiB (-0.03%)