use precise spans for recursive const evaluation #97740

RalfJung · 2022-06-04T19:54:53Z

This fixes #73283 by using a TyCtxtAt with a more precise span when the interpreter recursively calls itself. Hopefully such calls are sufficiently rare that this does not cost us too much performance.

(In theory, cycles can also arise through layout computation, as layout can depend on consts -- but layout computation happens all the time so we'd have to do something to not make this terrible for performance.)

rust-highfive · 2022-06-04T19:54:56Z

Some changes occured to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occured to the CTFE / Miri engine

cc @rust-lang/miri

rust-highfive · 2022-06-04T19:54:57Z

r? @lcnr

(rust-highfive has picked a reviewer for you, use r? to override)

RalfJung · 2022-06-04T19:56:08Z

@bors try @rust-timer queue

rust-timer · 2022-06-04T19:56:09Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2022-06-04T19:56:17Z

⌛ Trying commit 0db50505a2feb88eef2e3209969cd0df39bb9d6c with merge c9b99076039201534fb4a746e0b7b4b9bbe9f474...

RalfJung · 2022-06-04T19:57:28Z

compiler/rustc_const_eval/src/interpret/step.rs

    pub fn statement(&mut self, stmt: &mir::Statement<'tcx>) -> InterpResult<'tcx> {
        info!("{:?}", stmt);

        use rustc_middle::mir::StatementKind::*;

-        // Some statements (e.g., box) push new stack frames.


This is no longer true since the Box nullary op got removed. :)

RalfJung · 2022-06-04T19:58:08Z

compiler/rustc_const_eval/src/interpret/step.rs

            return Ok(true);
        }

        M::before_terminator(self)?;

        let terminator = basic_block.terminator();
-        assert_eq!(old_frames, self.frame_idx());


I have no idea what the point of this was; it is probably an ancient leftover.

That function is called by const_prop, where updating the index like that is totally meaningless.

RalfJung · 2022-06-04T20:23:43Z

@bors try @rust-timer queue

rust-timer · 2022-06-04T20:23:45Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2022-06-04T20:23:51Z

⌛ Trying commit 467e0f4 with merge 6a683121e42aa205b571a37453d7acaf44416332...

bors · 2022-06-04T21:59:42Z

☀️ Try build successful - checks-actions
Build commit: 6a683121e42aa205b571a37453d7acaf44416332 (6a683121e42aa205b571a37453d7acaf44416332)

rust-timer · 2022-06-04T21:59:43Z

Queued 6a683121e42aa205b571a37453d7acaf44416332 with parent 6364179, future comparison URL.

rust-timer · 2022-06-04T23:17:19Z

Finished benchmarking commit (6a683121e42aa205b571a37453d7acaf44416332): comparison url.

Instruction count

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

Max RSS (memory usage)

Results

Primary benchmarks: 😿 relevant regression found
Secondary benchmarks: 😿 relevant regression found

	mean¹	max	count²
Regressions 😿 (primary)	2.8%	2.8%	1
Regressions 😿 (secondary)	3.8%	3.8%	1
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	2.8%	2.8%	1

Cycles

Results

Primary benchmarks: no relevant changes found
Secondary benchmarks: 😿 relevant regressions found

	mean¹	max	count²
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	2.5%	2.5%	2
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	N/A	N/A	0

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

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.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

the arithmetic mean of the percent change ↩ ↩²
number of relevant changes ↩ ↩²

lcnr · 2022-06-08T08:30:40Z

@bors r+

bors · 2022-06-08T08:30:42Z

📌 Commit 467e0f4 has been approved by lcnr

bors · 2022-06-08T16:01:39Z

⌛ Testing commit 467e0f4 with merge a69f0b31afb62056cf6d506b5e6ffa3498292a7e...

bors · 2022-06-08T17:57:52Z

💔 Test failed - checks-actions

rust-log-analyzer · 2022-06-08T17:59:09Z

The job x86_64-apple-2 failed! Check out the build log: (web) (plain)

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

      Memory: 14 GB
      System Firmware Version: VMW71.00V.13989454.B64.1906190538
      Apple ROM Info: [MS_VM_CERT/SHA1/27d66596a61c48dd3dc7216fd715126e33f59ae7]Welcome to the Virtual Machine
      SMC Version (system): 2.8f0
      Serial Number (system): VM9Ydk3zZdBt
      Provisioning UDID: 4203018E-580F-C1B5-9525-B745CECA79EB

hw.ncpu: 3
hw.byteorder: 1234
---
failures:

---- [ui] src/test/ui/abi/anon-extern-mod.rs stdout ----

error: test compilation failed although it shouldn't!
status: signal: 9 (SIGKILL)
command: "/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage2/bin/rustc" "/Users/runner/work/rust/rust/src/test/ui/abi/anon-extern-mod.rs" "-Zthreads=1" "--target=x86_64-apple-darwin" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-C" "prefer-dynamic" "-o" "/Users/runner/work/rust/rust/build/x86_64-apple-darwin/test/ui/abi/anon-extern-mod/a" "-Crpath" "-O" "-Cdebuginfo=0" "-Lnative=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/native/rust-test-helpers" "-L" "/Users/runner/work/rust/rust/build/x86_64-apple-darwin/test/ui/abi/anon-extern-mod/auxiliary"
stdout: none
stderr: none

---- [ui] src/test/ui/abi/abi-sysv64-arg-passing.rs stdout ----

error: test compilation failed although it shouldn't!
error: test compilation failed although it shouldn't!
status: signal: 9 (SIGKILL)
command: "/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage2/bin/rustc" "/Users/runner/work/rust/rust/src/test/ui/abi/abi-sysv64-arg-passing.rs" "-Zthreads=1" "--target=x86_64-apple-darwin" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-C" "prefer-dynamic" "-o" "/Users/runner/work/rust/rust/build/x86_64-apple-darwin/test/ui/abi/abi-sysv64-arg-passing/a" "-Crpath" "-O" "-Cdebuginfo=0" "-Lnative=/Users/runner/work/rust/rust/build/x86_64-apple-darwin/native/rust-test-helpers" "-L" "/Users/runner/work/rust/rust/build/x86_64-apple-darwin/test/ui/abi/abi-sysv64-arg-passing/auxiliary"
stdout: none
stderr: none


failures:
    [ui] src/test/ui/abi/abi-sysv64-arg-passing.rs

RalfJung · 2022-06-08T18:06:55Z

---- [ui] src/test/ui/abi/anon-extern-mod.rs stdout ----
error: test compilation failed although it shouldn't!
status: signal: 9 (SIGKILL)

This cannot really be caused by this PR, right?
@bors retry apple builder failure in src/test/ui/abi/anon-extern-mod.rs

bors · 2022-06-09T01:52:18Z

⌛ Testing commit 467e0f4 with merge 282445a...

bors · 2022-06-09T04:32:56Z

☀️ Test successful - checks-actions
Approved by: lcnr
Pushing 282445a to master...

rust-timer · 2022-06-09T05:51:20Z

Finished benchmarking commit (282445a): comparison url.

Instruction count

Primary benchmarks: 🎉 relevant improvement found
Secondary benchmarks: 😿 relevant regression found

	mean¹	max	count²
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	0.3%	0.3%	1
Improvements 🎉 (primary)	-0.5%	-0.5%	1
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	-0.5%	-0.5%	1

Max RSS (memory usage)

Results

Primary benchmarks: no relevant changes found
Secondary benchmarks: 😿 relevant regressions found

	mean¹	max	count²
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	2.3%	2.3%	2
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	N/A	N/A	0

Cycles

Results

Primary benchmarks: 😿 relevant regressions found
Secondary benchmarks: 😿 relevant regressions found

	mean¹	max	count²
Regressions 😿 (primary)	2.4%	2.6%	2
Regressions 😿 (secondary)	4.2%	4.4%	2
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	2.4%	2.6%	2

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

the arithmetic mean of the percent change ↩ ↩² ↩³
number of relevant changes ↩ ↩² ↩³

tweak some bug!s

9e00fb0

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 4, 2022

rust-highfive assigned lcnr Jun 4, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 4, 2022

RalfJung mentioned this pull request Jun 4, 2022

More precise spans for CTFE cycle errors #73283

Closed

RalfJung force-pushed the ctfe-cycle-spans branch from 95588c7 to 0db5050 Compare June 4, 2022 19:55

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

RalfJung commented Jun 4, 2022

View reviewed changes

RalfJung added 2 commits June 4, 2022 15:59

Move statement_index increment out of statement() function

7892e1c

That function is called by const_prop, where updating the index like that is totally meaningless.

const_prop_lint: ensure we have up-to-date cur_span()

9ab4f87

This comment has been minimized.

Sign in to view

use precise spans for recursive const evaluation

467e0f4

RalfJung force-pushed the ctfe-cycle-spans branch from 0db5050 to 467e0f4 Compare June 4, 2022 20:18

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 4, 2022

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 8, 2022

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 8, 2022

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 8, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 9, 2022

bors merged commit 282445a into rust-lang:master Jun 9, 2022

rustbot added this to the 1.63.0 milestone Jun 9, 2022

RalfJung deleted the ctfe-cycle-spans branch June 9, 2022 13:16

use precise spans for recursive const evaluation #97740

use precise spans for recursive const evaluation #97740

Uh oh!

Conversation

RalfJung commented Jun 4, 2022

Uh oh!

rust-highfive commented Jun 4, 2022

Uh oh!

rust-highfive commented Jun 4, 2022

Uh oh!

RalfJung commented Jun 4, 2022

Uh oh!

rust-timer commented Jun 4, 2022

Uh oh!

bors commented Jun 4, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

RalfJung commented Jun 4, 2022

Uh oh!

rust-timer commented Jun 4, 2022

Uh oh!

bors commented Jun 4, 2022

Uh oh!

bors commented Jun 4, 2022

Uh oh!

rust-timer commented Jun 4, 2022

Uh oh!

rust-timer commented Jun 4, 2022

Footnotes

Uh oh!

lcnr commented Jun 8, 2022

Uh oh!

bors commented Jun 8, 2022

Uh oh!

bors commented Jun 8, 2022

Uh oh!

bors commented Jun 8, 2022

Uh oh!

rust-log-analyzer commented Jun 8, 2022

Uh oh!

RalfJung commented Jun 8, 2022

Uh oh!

bors commented Jun 9, 2022

Uh oh!

bors commented Jun 9, 2022

Uh oh!

rust-timer commented Jun 9, 2022

Footnotes

Uh oh!

Uh oh!