Don't codegen dead code #104860

jyn514 · 2022-11-25T04:16:30Z

This is the internal-facing portion of #103356; it should not be user-visible in any way, since we already stripped dead code at the linker level.

Fixes #104858.

rustbot · 2022-11-25T04:16:35Z

r? @davidtwco

(rustbot has picked a reviewer for you, use r? to override)

jyn514 · 2022-11-25T04:16:48Z

@bors try @rust-timer queue

bors · 2022-11-25T04:16:56Z

⌛ Trying commit 87ade9eb4e10ffec341aad7dc47bcac2b5c5256d with merge e3cd8ee648dc72d29df7fcfe928b572f5523d06a...

bors · 2022-11-25T04:33:54Z

💔 Test failed - checks-actions

This is the internal-facing portion of rust-lang#103356; it should not be user-visible in any way.

jyn514 · 2022-11-25T04:54:00Z

@bors try @rust-timer queue

bors · 2022-11-25T04:54:08Z

⌛ Trying commit d64517d with merge 50dbcf2ac6a27d114f1a1d61c861b0c986127002...

bjorn3 · 2022-11-25T06:15:37Z

compiler/rustc_monomorphize/src/collector.rs

+        if !live_symbols.contains(&def_id) && !self.tcx.sess.link_dead_code() {
+            // This is dead code; ignore it.
+            return;
+        }


Why would this condition even be true when not compiling in incr comp mode? Also won't this remove all global_asm!() usages?

Why would this condition even happen when not compiling in incr comp mode?

I've observed this to happen with every unreachable item.

rust/compiler/rustc_monomorphize/src/collector.rs

Lines 376 to 379 in d64517d

let crate_items = tcx.hir_crate_items(());

for id in crate_items.items() {

collector.process_item(id);

looks at every item in the crate regardless of whether it's reachable.

I hadn't considered global_asm, I can special-case it here if you think it's necessary.

Global_asm mut never be removed. Also please make this early exit conditional on the lazy collection mode. The eager collection mode doesn't omit anything on purpose to improve incr comp object file reuse.

rust-log-analyzer · 2022-11-25T06:40:54Z

The job x86_64-gnu-llvm-13 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

failures:

---- [codegen] src/test/codegen/debuginfo-generic-closure-env-names.rs stdout ----

error: verification with 'FileCheck' failed
status: exit status: 1
command: "/usr/lib/llvm-13/bin/FileCheck" "--input-file" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/debuginfo-generic-closure-env-names/debuginfo-generic-closure-env-names.ll" "/checkout/src/test/codegen/debuginfo-generic-closure-env-names.rs" "--allow-unused-prefixes" "--check-prefixes" "CHECK,NONMSVC"
stdout: none
--- stderr -------------------------------
/checkout/src/test/codegen/debuginfo-generic-closure-env-names.rs:26:11: error: CHECK: expected string not found in input
// CHECK: ![[generic_async_function_NAMESPACE:[0-9]+]] = !DINamespace(name: "generic_async_function"
          ^
/checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/debuginfo-generic-closure-env-names/debuginfo-generic-closure-env-names.ll:2170:57: note: scanning from here
!1000 = !DINamespace(name: "function_containing_closure", scope: !14)
                                                        ^
/checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/debuginfo-generic-closure-env-names/debuginfo-generic-closure-env-names.ll:2180:5: note: possible intended match here
!1010 = distinct !DISubprogram(name: "generic_async_function<debuginfo_generic_closure_env_names::Foo>", linkageName: "_RINvCs3mvmSMtNbaT_35debuginfo_generic_closure_env_names22generic_async_functionNtB2_3FooEB2_", scope: !14, file: !118, line: 68, type: !1011, scopeLine: 68, flags: DIFlagPrototyped, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, unit: !23, templateParams: !991, retainedNodes: !1013)

Input file: /checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/debuginfo-generic-closure-env-names/debuginfo-generic-closure-env-names.ll
Check file: /checkout/src/test/codegen/debuginfo-generic-closure-env-names.rs


-dump-input=help explains the following input dump.
Input was:
<<<<<<
            .
            .
            .
            .
         2165: !995 = !DILocation(line: 66, column: 2, scope: !986) 
         2166: !996 = distinct !DISubprogram(name: "function_containing_closure<u32>", linkageName: "_RINvCs3mvmSMtNbaT_35debuginfo_generic_closure_env_names27function_containing_closuremEB2_", scope: !14, file: !118, line: 59, type: !997, scopeLine: 59, flags: DIFlagPrototyped, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, unit: !23, templateParams: !1005, retainedNodes: !1003) 
         2167: !997 = !DISubroutineType(types: !998) 
         2168: !998 = !{!999, !179} 
         2169: !999 = !DICompositeType(tag: DW_TAG_structure_type, name: "{closure_env#0}<u32>", scope: !1000, file: !2, size: 32, align: 32, elements: !1001, templateParams: !18, identifier: "4abc8377f7bf03d89300ff28b5728df4") 
         2170: !1000 = !DINamespace(name: "function_containing_closure", scope: !14) 
check:26'0                                                             X~~~~~~~~~~~~~ error: no match found
         2171: !1001 = !{!1002} 
check:26'0     ~~~~~~~~~~~~~~~~~
         2172: !1002 = !DIDerivedType(tag: DW_TAG_member, name: "x", scope: !999, file: !2, baseType: !179, size: 32, align: 32) 
check:26'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         2173: !1003 = !{!1004} 
check:26'0     ~~~~~~~~~~~~~~~~~
         2174: !1004 = !DILocalVariable(name: "x", arg: 1, scope: !996, file: !118, line: 59, type: !179) 
check:26'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         2175: !1005 = !{!1006} 
check:26'0     ~~~~~~~~~~~~~~~~~
         2176: !1006 = !DITemplateTypeParameter(name: "T", type: !179) 
check:26'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         2177: !1007 = !DILocation(line: 59, column: 44, scope: !996) 
check:26'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         2178: !1008 = !DILocation(line: 65, column: 12, scope: !996) 
check:26'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         2179: !1009 = !DILocation(line: 66, column: 2, scope: !996) 
check:26'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         2180: !1010 = distinct !DISubprogram(name: "generic_async_function<debuginfo_generic_closure_env_names::Foo>", linkageName: "_RINvCs3mvmSMtNbaT_35debuginfo_generic_closure_env_names22generic_async_functionNtB2_3FooEB2_", scope: !14, file: !118, line: 68, type: !1011, scopeLine: 68, flags: DIFlagPrototyped, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, unit: !23, templateParams: !991, retainedNodes: !1013) 
check:26'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
check:26'1         ?                                                                                                                                                                                                                                                                                                                                                                                                                    possible intended match
         2181: !1011 = !DISubroutineType(types: !1012) 
check:26'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         2182: !1012 = !{!143, !17} 
check:26'0     ~~~~~~~~~~~~~~~~~~~~~
         2183: !1013 = !{!1014} 
check:26'0     ~~~~~~~~~~~~~~~~~
         2184: !1014 = !DILocalVariable(name: "x", arg: 1, scope: !1010, file: !118, line: 68, type: !17) 
check:26'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         2185: !1015 = !DILocation(line: 68, column: 45, scope: !1010) 
check:26'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            .
            .
>>>>>>
------------------------------------------

jruderman · 2022-11-25T06:50:39Z

Does this affect code coverage of the test suite?

bors · 2022-11-25T07:34:30Z

☀️ Try build successful - checks-actions
Build commit: 50dbcf2ac6a27d114f1a1d61c861b0c986127002 (50dbcf2ac6a27d114f1a1d61c861b0c986127002)

rust-timer · 2022-11-25T09:38:35Z

Finished benchmarking commit (50dbcf2ac6a27d114f1a1d61c861b0c986127002): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf +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.8%	[0.4%, 1.4%]	31
Regressions ❌ (secondary)	1.4%	[0.3%, 2.5%]	15
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.8%	[0.4%, 1.4%]	31

Max RSS (memory usage)

Results

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)	1.4%	[0.8%, 2.2%]	5
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.4%	[-2.0%, -0.8%]	2
Improvements ✅ (secondary)	-3.0%	[-3.4%, -2.6%]	2
All ❌✅ (primary)	0.6%	[-2.0%, 2.2%]	7

Cycles

Results

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)	2.7%	[2.3%, 3.2%]	3
Regressions ❌ (secondary)	2.5%	[2.4%, 2.7%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.7%	[2.3%, 3.2%]	3

tmiasko · 2022-11-25T11:12:40Z

In the existing implementation, dead code is already omitted based on the results of reachability pass in compiler/rustc_passes/src/reachable.rs whose results are available as is_reachable_non_generic query. The root mono item collectors skip unreachable definitions based on condition in push_if_root -> is_root -> is_reachable_non_generic.

jyn514 · 2022-11-28T20:55:52Z

Ah, thanks! looks like I was putting the mir_borrowck call in the wrong place in #103356, it should go in is_instantiable instead, not process_item.

This would have avoided a bug in rust-lang#104860. In practice this shouldn't matter since nothing uses the query other than the `dead_code` lint, but this isn't documented as an internal-only query so it seems nice for it to be accurate. I think for `dead_code` it doesn't matter because the relevant code is generated by `rustc_builtin_macros` and isn't linted.

…miasko Mark `proc_macro_decls_static` as always used This would have avoided a bug in rust-lang#104860. In practice this shouldn't matter since nothing uses the query other than the `dead_code` lint, but this isn't documented as an internal-only query so it seems nice for it to be accurate. I think for `dead_code` it doesn't matter because the relevant code is generated by `rustc_builtin_macros` and isn't linted. I think `@tmiasko` or `@bjorn3` would be a good reviewer? r? `@tmiasko`

rustbot assigned davidtwco Nov 25, 2022

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 25, 2022

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Nov 25, 2022

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 25, 2022

jyn514 force-pushed the no-codegen-dead-code branch from 87ade9e to 4219733 Compare November 25, 2022 04:41

Don't codegen dead code

d64517d

This is the internal-facing portion of rust-lang#103356; it should not be user-visible in any way.

jyn514 force-pushed the no-codegen-dead-code branch from 4219733 to d64517d Compare November 25, 2022 04:53

This comment has been minimized.

Sign in to view

bjorn3 reviewed Nov 25, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

rustbot added perf-regression Performance regression. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Nov 25, 2022

jyn514 closed this Nov 28, 2022

jyn514 mentioned this pull request Nov 28, 2022

Don't codegen items that are statically unreachable #104858

Closed

jyn514 mentioned this pull request Dec 20, 2022

Mark proc_macro_decls_static as always used #105978

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't codegen dead code #104860

Don't codegen dead code #104860

jyn514 commented Nov 25, 2022 •

edited

Loading

rustbot commented Nov 25, 2022

jyn514 commented Nov 25, 2022

This comment has been minimized.

bors commented Nov 25, 2022

bors commented Nov 25, 2022

jyn514 commented Nov 25, 2022

This comment has been minimized.

bors commented Nov 25, 2022

This comment has been minimized.

bjorn3 Nov 25, 2022 •

edited

Loading

jyn514 Nov 25, 2022 •

edited

Loading

bjorn3 Nov 25, 2022

rust-log-analyzer commented Nov 25, 2022

jruderman commented Nov 25, 2022

bors commented Nov 25, 2022

This comment has been minimized.

rust-timer commented Nov 25, 2022

tmiasko commented Nov 25, 2022

jyn514 commented Nov 28, 2022

	let crate_items = tcx.hir_crate_items(());

	for id in crate_items.items() {
	collector.process_item(id);

Don't codegen dead code #104860

Don't codegen dead code #104860

Conversation

jyn514 commented Nov 25, 2022 • edited Loading

rustbot commented Nov 25, 2022

jyn514 commented Nov 25, 2022

This comment has been minimized.

bors commented Nov 25, 2022

bors commented Nov 25, 2022

jyn514 commented Nov 25, 2022

This comment has been minimized.

bors commented Nov 25, 2022

This comment has been minimized.

bjorn3 Nov 25, 2022 • edited Loading

Choose a reason for hiding this comment

jyn514 Nov 25, 2022 • edited Loading

Choose a reason for hiding this comment

bjorn3 Nov 25, 2022

Choose a reason for hiding this comment

rust-log-analyzer commented Nov 25, 2022

jruderman commented Nov 25, 2022

bors commented Nov 25, 2022

This comment has been minimized.

rust-timer commented Nov 25, 2022

Overall result: ❌ regressions - ACTION NEEDED

Instruction count

Max RSS (memory usage)

Cycles

tmiasko commented Nov 25, 2022

jyn514 commented Nov 28, 2022

jyn514 commented Nov 25, 2022 •

edited

Loading

bjorn3 Nov 25, 2022 •

edited

Loading

jyn514 Nov 25, 2022 •

edited

Loading