Skip to content

Correct codegen of ConstValue::Indirect scalar and scalar pair #116102

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Sep 26, 2023

Conversation

cjgillot
Copy link
Contributor

This concerns 3 tricky cases with ConstValue::Indirect:

  • if we want a non-pointer scalar;
  • if we have non-zero offset;
  • if offset points to uninit memory => generate poison instead of an ICE. This case could happen in unreachable code, trying to extract a field from the wrong variant.

Those cases are not currently emitted by the compiler, but are exercised by #116012.

@rustbot
Copy link
Collaborator

rustbot commented Sep 23, 2023

r? @oli-obk

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

@rustbot 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 Sep 23, 2023
@oli-obk
Copy link
Contributor

oli-obk commented Sep 26, 2023

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Sep 26, 2023

📌 Commit ac0683b has been approved by oli-obk

It is now in the queue for this repository.

@bors 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 Sep 26, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 26, 2023
Correct codegen of `ConstValue::Indirect` scalar and scalar pair

This concerns 3 tricky cases with `ConstValue::Indirect`:
- if we want a non-pointer scalar;
- if we have non-zero offset;
- if offset points to uninit memory => generate `poison` instead of an ICE. This case could happen in unreachable code, trying to extract a field from the wrong variant.

Those cases are not currently emitted by the compiler, but are exercised by rust-lang#116012.
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 26, 2023
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#116099 (Add regression test for issue rust-lang#79865)
 - rust-lang#116102 (Correct codegen of `ConstValue::Indirect` scalar and scalar pair)
 - rust-lang#116131 (Rename `cold_path` to `outline`)
 - rust-lang#116144 (subst -> instantiate)
 - rust-lang#116151 (Fix typo in rustdoc unstable features doc)
 - rust-lang#116153 (Update books)
 - rust-lang#116162 (Gate and validate `#[rustc_safe_intrinsic]`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 26, 2023
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#116099 (Add regression test for issue rust-lang#79865)
 - rust-lang#116102 (Correct codegen of `ConstValue::Indirect` scalar and scalar pair)
 - rust-lang#116131 (Rename `cold_path` to `outline`)
 - rust-lang#116144 (subst -> instantiate)
 - rust-lang#116151 (Fix typo in rustdoc unstable features doc)
 - rust-lang#116153 (Update books)
 - rust-lang#116162 (Gate and validate `#[rustc_safe_intrinsic]`)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr
Copy link
Member

@bors rollup=iffy

@bors
Copy link
Collaborator

bors commented Sep 26, 2023

⌛ Testing commit ac0683b with merge 5899a80...

@bors
Copy link
Collaborator

bors commented Sep 26, 2023

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 5899a80 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 26, 2023
@bors bors merged commit 5899a80 into rust-lang:master Sep 26, 2023
@rustbot rustbot added this to the 1.74.0 milestone Sep 26, 2023
@cjgillot cjgillot deleted the indirect-scalar branch September 26, 2023 21:08
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (5899a80): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

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.2% [1.2%, 1.2%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 1.2% [1.2%, 1.2%] 1

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: 632.697s -> 633.132s (0.07%)
Artifact size: 317.03 MiB -> 317.17 MiB (0.04%)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants