Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

abi: add AddressSpace field to Primitive::Pointer #107248

Merged
merged 3 commits into from
Jan 26, 2023

Conversation

erikdesjardins
Copy link
Contributor

...and remove it from PointeeInfo, which isn't meant for this.

There are still various places (marked with FIXMEs) that assume all pointers
have the same size and alignment. Fixing this requires parsing non-default
address spaces in the data layout string (and various other changes),
which will be done in a followup.
(That is, if it's actually worth it to support multiple different pointer sizes.
There is a lot of code that would be affected by that.)

Fixes #106367

r? @oli-obk
cc @Patryk27

there were fixmes for this already

i am about to remove is_ptr (since callers need to properly distinguish
between pointers in different address spaces), so might as well do this
at the same time
...and remove it from `PointeeInfo`, which isn't meant for this.

There are still various places (marked with FIXMEs) that assume all pointers
have the same size and alignment. Fixing this requires parsing non-default
address spaces in the data layout string, which will be done in a followup.
@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 Jan 24, 2023
@rustbot
Copy link
Collaborator

rustbot commented Jan 24, 2023

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Copy link
Contributor

@oli-obk oli-obk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like that snippet about GlobalAlloc is only ever used once. A bit surprising, but maybe we'll see more of it if we end up teaching const eval about address spaces (it already kind of has some of them implicitly)

compiler/rustc_codegen_gcc/src/consts.rs Outdated Show resolved Hide resolved
@oli-obk
Copy link
Contributor

oli-obk commented Jan 24, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Jan 24, 2023

📌 Commit 009192b 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 Jan 24, 2023
@rustbot
Copy link
Collaborator

rustbot commented Jan 25, 2023

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

@oli-obk
Copy link
Contributor

oli-obk commented Jan 25, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Jan 25, 2023

📌 Commit adc1890 has been approved by oli-obk

It is now in the queue for this repository.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 25, 2023
abi: add AddressSpace field to Primitive::Pointer

...and remove it from `PointeeInfo`, which isn't meant for this.

There are still various places (marked with FIXMEs) that assume all pointers
have the same size and alignment. Fixing this requires parsing non-default
address spaces in the data layout string (and various other changes),
which will be done in a followup.
(That is, if it's actually worth it to support multiple different pointer sizes.
There is a lot of code that would be affected by that.)

Fixes rust-lang#106367

r? `@oli-obk`
cc `@Patryk27`
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 26, 2023
…iaskrgr

Rollup of 11 pull requests

Successful merges:

 - rust-lang#106407 (Improve proc macro attribute diagnostics)
 - rust-lang#106960 (Teach parser to understand fake anonymous enum syntax)
 - rust-lang#107085 (Custom MIR: Support binary and unary operations)
 - rust-lang#107086 (Print PID holding bootstrap build lock on Linux)
 - rust-lang#107175 (Fix escaping inference var ICE in `point_at_expr_source_of_inferred_type`)
 - rust-lang#107204 (suggest qualifying bare associated constants)
 - rust-lang#107248 (abi: add AddressSpace field to Primitive::Pointer )
 - rust-lang#107272 (Implement ObjectSafe and WF in the new solver)
 - rust-lang#107285 (Implement `Generator` and `Future` in the new solver)
 - rust-lang#107286 (ICE in new solver if we see an inference variable)
 - rust-lang#107313 (Add Style Team Triagebot config)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a8b5e5d into rust-lang:master Jan 26, 2023
@rustbot rustbot added this to the 1.69.0 milestone Jan 26, 2023
@erikdesjardins erikdesjardins deleted the addrspace branch January 26, 2023 18:27
@@ -1468,7 +1461,6 @@ pub struct PointeeInfo {
pub size: Size,
pub align: Align,
pub safe: Option<PointerKind>,
pub address_space: AddressSpace,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks a ton. :)
Note to self: add a comment to PointeeInfo stating that this is for optimization purposes only and safe to ignore by backends (as suggested by @eddyb)

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jan 31, 2023
PointeeInfo is advisory only

rust-lang#107248 fixed PointeeInfo being used in ways that don't actually work. Hopefully this comments helps avoid such issues in the future.

Cc `@eddyb`
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jan 31, 2023
PointeeInfo is advisory only

rust-lang#107248 fixed PointeeInfo being used in ways that don't actually work. Hopefully this comments helps avoid such issues in the future.

Cc ``@eddyb``
bjorn3 pushed a commit to bjorn3/rust that referenced this pull request Feb 9, 2023
…iaskrgr

Rollup of 11 pull requests

Successful merges:

 - rust-lang#106407 (Improve proc macro attribute diagnostics)
 - rust-lang#106960 (Teach parser to understand fake anonymous enum syntax)
 - rust-lang#107085 (Custom MIR: Support binary and unary operations)
 - rust-lang#107086 (Print PID holding bootstrap build lock on Linux)
 - rust-lang#107175 (Fix escaping inference var ICE in `point_at_expr_source_of_inferred_type`)
 - rust-lang#107204 (suggest qualifying bare associated constants)
 - rust-lang#107248 (abi: add AddressSpace field to Primitive::Pointer )
 - rust-lang#107272 (Implement ObjectSafe and WF in the new solver)
 - rust-lang#107285 (Implement `Generator` and `Future` in the new solver)
 - rust-lang#107286 (ICE in new solver if we see an inference variable)
 - rust-lang#107313 (Add Style Team Triagebot config)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
qinheping added a commit to qinheping/kani that referenced this pull request Mar 9, 2023
(GeneratorWitnessMIR)
Compute generator saved locals on MIR rust-lang/rust#101692

(ParamEnv)
InstCombine away intrinsic validity assertions rust-lang/rust#105582

(Primitive::Pointer) abi: add AddressSpace field to Primitive::Pointer  rust-lang/rust#107248
qinheping added a commit to qinheping/kani that referenced this pull request Mar 9, 2023
(GeneratorWitnessMIR)
Compute generator saved locals on MIR rust-lang/rust#101692

(ParamEnv)
InstCombine away intrinsic validity assertions rust-lang/rust#105582

(Primitive::Pointer) abi: add AddressSpace field to Primitive::Pointer  rust-lang/rust#107248
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

function pointer address space handling (for AVR) is wrong
5 participants