cleanup: remove pointee types #105545

erikdesjardins · 2022-12-11T01:29:30Z

This can't be merged until the oldest LLVM version we support uses opaque pointers, which will be the case after #114148. (Also note -Cllvm-args="-opaque-pointers=0" can technically be used in LLVM 15, though I don't think we should support that configuration.)

I initially hoped this would provide some minor perf win, but in #105412 (comment) it had very little impact, so this is only valuable as a cleanup.

As a followup, this will enable #96242 to be resolved.

r? @ghost

@rustbot label S-blocked

erikdesjardins · 2022-12-11T01:30:14Z

@rustbot label -S-waiting-on-review

bjorn3 · 2022-12-11T17:26:30Z

compiler/rustc_codegen_llvm/src/builder.rs

@@ -1457,7 +1441,6 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
            return;
        }

-        let ptr = self.pointercast(ptr, self.cx.type_i8p());


Can pointercast be implemented as nop after these changes?

It can still be used to convert between different pointer address spaces, e.g. when casting data pointers to function pointers on harvard architecture targets: https://godbolt.org/z/c4nEb8sTW. (Note that we don't use pointercast for this today; that IR is actually invalid as a result. I'll open a PR to fix it.)

On other targets it'll always do nothing. The LLVM side has a check to avoid creating a new instruction if the pointer is already the correct type.

compiler/rustc_codegen_llvm/src/callee.rs

…orn3 Fix transmutes between pointers in different address spaces (e.g. fn ptrs on AVR) Currently, this causes a verifier error (https://godbolt.org/z/YYohed4bj), since it uses `bitcast`, which can't convert between address spaces. Uncovered due to rust-lang#105545 (comment) r? `@bjorn3`

bors · 2022-12-23T06:36:09Z

☔ The latest upstream changes (presumably #106070) made this pull request unmergeable. Please resolve the merge conflicts.

Operand::extract_field: only cast llval if it's a pointer and replace bitcast w/ pointercast. Fixes rust-lang#105439. Also cc `@erikdesjardins,` looks like another place to cleanup as part of rust-lang#105545

bors · 2023-07-29T16:58:59Z

☔ The latest upstream changes (presumably #114148) made this pull request unmergeable. Please resolve the merge conflicts.

rustbot · 2023-07-29T17:51:03Z

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo

erikdesjardins · 2023-07-29T17:51:12Z

r? @bjorn3

@rustbot review
@rustbot label -S-blocked

bjorn3

r=me for the LLVM backend with review comments fixed. @antoyo could you review the GCC backend changes?

compiler/rustc_codegen_llvm/src/builder.rs

compiler/rustc_codegen_llvm/src/intrinsic.rs

compiler/rustc_codegen_llvm/src/type_of.rs

With opaque pointers, there's no longer a need to generate a chain of pointer types in the intrinsic name when arguments are pointers to pointers.

antoyo

It looks good to me.
If there's anything to fix, I'll do it by myself.

bjorn3 · 2023-08-01T18:34:29Z

@bors r+ rollup=never

bors · 2023-08-01T18:34:31Z

📌 Commit 5580012 has been approved by bjorn3

It is now in the queue for this repository.

bors · 2023-08-01T19:44:21Z

⌛ Testing commit 5580012 with merge abd3637...

bors · 2023-08-01T21:27:42Z

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

rust-timer · 2023-08-02T05:09:04Z

Finished benchmarking commit (abd3637): 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.4%	[-0.4%, -0.4%]	2
Improvements ✅ (secondary)	-0.4%	[-0.5%, -0.3%]	2
All ❌✅ (primary)	-0.4%	[-0.4%, -0.4%]	2

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)	-	-	0
Regressions ❌ (secondary)	2.2%	[2.0%, 2.4%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

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

Binary size

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

Bootstrap: missing data

Language -------- - [Uplift `clippy::fn_null_check` lint as `useless_ptr_null_checks`.] (rust-lang/rust#111717) - [Make `noop_method_call` warn by default.] (rust-lang/rust#111916) - [Support interpolated block for `try` and `async` in macros.] (rust-lang/rust#112953) - [Make `unconditional_recursion` lint detect recursive drops.] (rust-lang/rust#113902) - [Future compatibility warning for some impls being incorrectly considered not overlapping.] (rust-lang/rust#114023) - [The `invalid_reference_casting` lint is now **deny-by-default** (instead of allow-by-default)] (rust-lang/rust#112431) Compiler -------- - [Write version information in a `.comment` section like GCC/Clang.] (rust-lang/rust#97550) - [Add documentation on v0 symbol mangling.] (rust-lang/rust#97571) - [Stabilize `extern "thiscall"` and `"thiscall-unwind"` ABIs.] (rust-lang/rust#114562) - [Only check outlives goals on impl compared to trait.] (rust-lang/rust#109356) - [Infer type in irrefutable slice patterns with fixed length as array.] (rust-lang/rust#113199) - [Discard default auto trait impls if explicit ones exist.] (rust-lang/rust#113312) - Add several new tier 3 targets: - [`aarch64-unknown-teeos`] (rust-lang/rust#113480) - [`csky-unknown-linux-gnuabiv2`] (rust-lang/rust#113658) - [`riscv64-linux-android`] (rust-lang/rust#112858) - [`riscv64gc-unknown-hermit`] (rust-lang/rust#114004) - [`x86_64-unikraft-linux-musl`] (rust-lang/rust#113411) - [`x86_64-unknown-linux-ohos`] (rust-lang/rust#113061) - [Add `wasm32-wasi-preview1-threads` as a tier 2 target.] (rust-lang/rust#112922) Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. Libraries --------- - [Add `Read`, `Write` and `Seek` impls for `Arc<File>`.] (rust-lang/rust#94748) - [Merge functionality of `io::Sink` into `io::Empty`.] (rust-lang/rust#98154) - [Implement `RefUnwindSafe` for `Backtrace`] (rust-lang/rust#100455) - [Make `ExitStatus` implement `Default`] (rust-lang/rust#106425) - [`impl SliceIndex<str> for (Bound<usize>, Bound<usize>)`] (rust-lang/rust#111081) - [Change default panic handler message format.] (rust-lang/rust#112849) - [Cleaner `assert_eq!` & `assert_ne!` panic messages.] (rust-lang/rust#111071) - [Correct the (deprecated) Android `stat` struct definitions.] (rust-lang/rust#113130) Stabilized APIs --------------- - [Unsigned `{integer}::div_ceil`] (https://doc.rust-lang.org/stable/std/primitive.u32.html#method.div_ceil) - [Unsigned `{integer}::next_multiple_of`] (https://doc.rust-lang.org/stable/std/primitive.u32.html#method.next_multiple_of) - [Unsigned `{integer}::checked_next_multiple_of`] (https://doc.rust-lang.org/stable/std/primitive.u32.html#method.checked_next_multiple_of) - [`std::ffi::FromBytesUntilNulError`] (https://doc.rust-lang.org/stable/std/ffi/struct.FromBytesUntilNulError.html) - [`std::os::unix::fs::chown`] (https://doc.rust-lang.org/stable/std/os/unix/fs/fn.chown.html) - [`std::os::unix::fs::fchown`] (https://doc.rust-lang.org/stable/std/os/unix/fs/fn.fchown.html) - [`std::os::unix::fs::lfchown`] (https://doc.rust-lang.org/stable/std/os/unix/fs/fn.lchown.html) - [`LocalKey::<Cell<T>>::get`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.get) - [`LocalKey::<Cell<T>>::set`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.set) - [`LocalKey::<Cell<T>>::take`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.take) - [`LocalKey::<Cell<T>>::replace`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.replace) - [`LocalKey::<RefCell<T>>::with_borrow`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.with_borrow) - [`LocalKey::<RefCell<T>>::with_borrow_mut`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.with_borrow_mut) - [`LocalKey::<RefCell<T>>::set`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.set-1) - [`LocalKey::<RefCell<T>>::take`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.take-1) - [`LocalKey::<RefCell<T>>::replace`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.replace-1) These APIs are now stable in const contexts: - [`rc::Weak::new`] (https://doc.rust-lang.org/stable/alloc/rc/struct.Weak.html#method.new) - [`sync::Weak::new`] (https://doc.rust-lang.org/stable/alloc/sync/struct.Weak.html#method.new) - [`NonNull::as_ref`] (https://doc.rust-lang.org/stable/core/ptr/struct.NonNull.html#method.as_ref) Cargo ----- - [Encode URL params correctly for `SourceId` in `Cargo.lock`.] (rust-lang/cargo#12280) - [Bail out an error when using `cargo::` in custom build script.] (rust-lang/cargo#12332) Compatibility Notes ------------------- - [Update the minimum external LLVM to 15.] (rust-lang/rust#114148) - [Check for non-defining uses of return position `impl Trait`.] (rust-lang/rust#112842) Internal Changes ---------------- These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools. - [Remove LLVM pointee types, supporting only opaque pointers.] (rust-lang/rust#105545) - [Port PGO/LTO/BOLT optimized build pipeline to Rust.] (rust-lang/rust#112235) - [Replace in-tree `rustc_apfloat` with the new version of the crate.] (rust-lang/rust#113843) - [Update to LLVM 17.] (rust-lang/rust#114048) - [Add `internal_features` lint for internal unstable features.] (rust-lang/rust#108955) - [Mention style for new syntax in tracking issue template.] (rust-lang/rust#113586)

cleanup: remove pointee types This can't be merged until the oldest LLVM version we support uses opaque pointers, which will be the case after rust-lang#114148. (Also note `-Cllvm-args="-opaque-pointers=0"` can technically be used in LLVM 15, though I don't think we should support that configuration.) I initially hoped this would provide some minor perf win, but in rust-lang#105412 (comment) it had very little impact, so this is only valuable as a cleanup. As a followup, this will enable rust-lang#96242 to be resolved. r? `@ghost` `@rustbot` label S-blocked

Pkgsrc changes: * Adjust patches and cargo checksums to new versions. * For an external LLVM, set dependency of llvm >= 15, in accordance with the upstream changes. * Add a patch with a backport from LLVM 17.0.3 fixing codegen for PPC, ref. rust-lang/rust#116845 Upstream changes: Version 1.73.0 (2023-10-05) ========================== Language -------- - [Uplift `clippy::fn_null_check` lint as `useless_ptr_null_checks`.] (rust-lang/rust#111717) - [Make `noop_method_call` warn by default.] (rust-lang/rust#111916) - [Support interpolated block for `try` and `async` in macros.] (rust-lang/rust#112953) - [Make `unconditional_recursion` lint detect recursive drops.] (rust-lang/rust#113902) - [Future compatibility warning for some impls being incorrectly considered not overlapping.] (rust-lang/rust#114023) - [The `invalid_reference_casting` lint is now **deny-by-default** (instead of allow-by-default)] (rust-lang/rust#112431 Compiler -------- - [Write version information in a `.comment` section like GCC/Clang.] (rust-lang/rust#97550) - [Add documentation on v0 symbol mangling.] (rust-lang/rust#97571) - [Stabilize `extern "thiscall"` and `"thiscall-unwind"` ABIs.] (rust-lang/rust#114562) - [Only check outlives goals on impl compared to trait.] (rust-lang/rust#109356) - [Infer type in irrefutable slice patterns with fixed length as array.] (rust-lang/rust#113199) - [Discard default auto trait impls if explicit ones exist.] (rust-lang/rust#113312) - Add several new tier 3 targets: - [`aarch64-unknown-teeos`] (rust-lang/rust#113480) - [`csky-unknown-linux-gnuabiv2`] (rust-lang/rust#113658) - [`riscv64-linux-android`] (rust-lang/rust#112858) - [`riscv64gc-unknown-hermit`] (rust-lang/rust#114004) - [`x86_64-unikraft-linux-musl`] (rust-lang/rust#113411) - [`x86_64-unknown-linux-ohos`] (rust-lang/rust#113061) - [Add `wasm32-wasi-preview1-threads` as a tier 2 target.] (rust-lang/rust#112922) Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. Libraries --------- - [Add `Read`, `Write` and `Seek` impls for `Arc<File>`.] (rust-lang/rust#94748) - [Merge functionality of `io::Sink` into `io::Empty`.] (rust-lang/rust#98154) - [Implement `RefUnwindSafe` for `Backtrace`] (rust-lang/rust#100455) - [Make `ExitStatus` implement `Default`] (rust-lang/rust#106425) - [`impl SliceIndex<str> for (Bound<usize>, Bound<usize>)`] (rust-lang/rust#111081) - [Change default panic handler message format.] (rust-lang/rust#112849) - [Cleaner `assert_eq!` & `assert_ne!` panic messages.] (rust-lang/rust#111071) - [Correct the (deprecated) Android `stat` struct definitions.] (rust-lang/rust#113130) Stabilized APIs --------------- - [Unsigned `{integer}::div_ceil`] (https://doc.rust-lang.org/stable/std/primitiv e.u32.html#method.div_ceil) - [Unsigned `{integer}::next_multiple_of`] (https://doc.rust-lang.org/stable/std/primitive.u32.html#method.next_multiple_of) - [Unsigned `{integer}::checked_next_multiple_of`] (https://doc.rust-lang.org/stable/std/primitive.u32.html#method.checked_next_multiple_of) - [`std::ffi::FromBytesUntilNulError`] (https://doc.rust-lang.org/stable/std/ffi/struct.FromBytesUntilNulError.html) - [`std::os::unix::fs::chown`] (https://doc.rust-lang.org/stable/std/os/unix/fs/fn.chown.html) - [`std::os::unix::fs::fchown`] (https://doc.rust-lang.org/stable/std/os/unix/fs/fn.fchown.html) - [`std::os::unix::fs::lfchown`] (https://doc.rust-lang.org/stable/std/os/unix/fs/fn.lchown.html) - [`LocalKey::<Cell<T>>::get`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.get) - [`LocalKey::<Cell<T>>::set`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.set) - [`LocalKey::<Cell<T>>::take`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.take) - [`LocalKey::<Cell<T>>::replace`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.replace) - [`LocalKey::<RefCell<T>>::with_borrow`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.with_borrow) - [`LocalKey::<RefCell<T>>::with_borrow_mut`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.with_borrow_mut) - [`LocalKey::<RefCell<T>>::set`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.set-1) - [`LocalKey::<RefCell<T>>::take`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.take-1) - [`LocalKey::<RefCell<T>>::replace`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.replace-1) These APIs are now stable in const contexts: - [`rc::Weak::new`] (https://doc.rust-lang.org/stable/alloc/rc/struct.Weak.html#method.new) - [`sync::Weak::new`] (https://doc.rust-lang.org/stable/alloc/sync/struct.Weak.html#method.new) - [`NonNull::as_ref`] (https://doc.rust-lang.org/stable/core/ptr/struct.NonNull.html#method.as_ref) Cargo ----- - [Encode URL params correctly for `SourceId` in `Cargo.lock`.] (rust-lang/cargo#12280) - [Bail out an error when using `cargo::` in custom build script.] (rust-lang/cargo#12332) Misc ---- Compatibility Notes ------------------- - [Update the minimum external LLVM to 15.] (rust-lang/rust#114148) - [Check for non-defining uses of return position `impl Trait`.] (rust-lang/rust#112842) Internal Changes ---------------- These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools. - [Remove LLVM pointee types, supporting only opaque pointers.] (rust-lang/rust#105545) - [Port PGO/LTO/BOLT optimized build pipeline to Rust.] (rust-lang/rust#112235) - [Replace in-tree `rustc_apfloat` with the new version of the crate.] (rust-lang/rust#113843) - [Update to LLVM 17.] (rust-lang/rust#114048) - [Add `internal_features` lint for internal unstable features.] (rust-lang/rust#108955) - [Mention style for new syntax in tracking issue template.] (rust-lang/rust#113586)

cleanup: remove zero-offset GEP This GEP would've been used to change the pointer type in the past, but after opaque pointers it's a no-op. I missed removing this in rust-lang#105545. Split out from rust-lang#121577.

Rollup merge of rust-lang#122051 - erikdesjardins:cleanup, r=nikic cleanup: remove zero-offset GEP This GEP would've been used to change the pointer type in the past, but after opaque pointers it's a no-op. I missed removing this in rust-lang#105545. Split out from rust-lang#121577.

rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 11, 2022

bjorn3 reviewed Dec 11, 2022

View reviewed changes

compiler/rustc_codegen_llvm/src/callee.rs Show resolved Hide resolved

erikdesjardins mentioned this pull request Dec 11, 2022

Fix transmutes between pointers in different address spaces (e.g. fn ptrs on AVR) #105578

Merged

This comment has been minimized.

Sign in to view

luqmana mentioned this pull request Dec 12, 2022

Operand::extract_field: only cast llval if it's a pointer and replace bitcast w/ pointercast. #105583

Merged

erikdesjardins force-pushed the ptrclean branch from 20a39b4 to 57f7f6a Compare July 28, 2023 21:39

This comment has been minimized.

Sign in to view

erikdesjardins force-pushed the ptrclean branch from 57f7f6a to 4ad4307 Compare July 28, 2023 21:53

This comment has been minimized.

Sign in to view

erikdesjardins changed the title ~~cleanup: remove pointee types from LLVM backend~~ cleanup: remove pointee types Jul 29, 2023

erikdesjardins force-pushed the ptrclean branch from 4ad4307 to 1ff68e9 Compare July 29, 2023 00:59

This comment has been minimized.

Sign in to view

erikdesjardins added 2 commits July 29, 2023 13:18

cg_llvm: remove pointee types and pointercast/bitcast-of-ptr

b654077

cg_ssa: remove pointee types and pointercast/bitcast-of-ptr

04303cf

erikdesjardins force-pushed the ptrclean branch from 1ff68e9 to 04303cf Compare July 29, 2023 17:19

erikdesjardins marked this pull request as ready for review July 29, 2023 17:51

rustbot assigned bjorn3 Jul 29, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels Jul 29, 2023

bjorn3 reviewed Jul 29, 2023

View reviewed changes

compiler/rustc_codegen_llvm/src/builder.rs Outdated Show resolved Hide resolved

compiler/rustc_codegen_llvm/src/intrinsic.rs Outdated Show resolved Hide resolved

compiler/rustc_codegen_llvm/src/type_of.rs Outdated Show resolved Hide resolved

erikdesjardins added 3 commits July 29, 2023 16:31

cg_llvm: inline check_store

def44c5

cg_llvm: clean up match

cf7788d

cg_llvm: simplify llvm.masked.gather/scatter naming with opaque pointers

5580012

With opaque pointers, there's no longer a need to generate a chain of pointer types in the intrinsic name when arguments are pointers to pointers.

antoyo approved these changes Aug 1, 2023

View reviewed changes

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 Aug 1, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 1, 2023

bors merged commit abd3637 into rust-lang:master Aug 1, 2023
12 checks passed

rustbot added this to the 1.73.0 milestone Aug 1, 2023

This was referenced Aug 1, 2023

Consistently use the highest bit of vector masks when converting to i1 vectors #104693

Open

Add Minimal Std implementation for UEFI #105861

Merged

Extract a create_wrapper_function for use in allocator shim writing #113722

Merged

erikdesjardins deleted the ptrclean branch August 1, 2023 23:22

erikdesjardins mentioned this pull request Mar 6, 2024

cleanup: remove zero-offset GEP #122051

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cleanup: remove pointee types #105545

cleanup: remove pointee types #105545

erikdesjardins commented Dec 11, 2022 •

edited

erikdesjardins commented Dec 11, 2022

bjorn3 Dec 11, 2022

erikdesjardins Dec 11, 2022

This comment has been minimized.

bors commented Dec 23, 2022

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bors commented Jul 29, 2023

rustbot commented Jul 29, 2023

erikdesjardins commented Jul 29, 2023

bjorn3 left a comment

antoyo left a comment

bjorn3 commented Aug 1, 2023

bors commented Aug 1, 2023

bors commented Aug 1, 2023

bors commented Aug 1, 2023

rust-timer commented Aug 2, 2023

cleanup: remove pointee types #105545

cleanup: remove pointee types #105545

Conversation

erikdesjardins commented Dec 11, 2022 • edited

erikdesjardins commented Dec 11, 2022

bjorn3 Dec 11, 2022

Choose a reason for hiding this comment

erikdesjardins Dec 11, 2022

Choose a reason for hiding this comment

This comment has been minimized.

bors commented Dec 23, 2022

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bors commented Jul 29, 2023

rustbot commented Jul 29, 2023

erikdesjardins commented Jul 29, 2023

bjorn3 left a comment

Choose a reason for hiding this comment

antoyo left a comment

Choose a reason for hiding this comment

bjorn3 commented Aug 1, 2023

bors commented Aug 1, 2023

bors commented Aug 1, 2023

bors commented Aug 1, 2023

rust-timer commented Aug 2, 2023

Overall result: ✅ improvements - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

erikdesjardins commented Dec 11, 2022 •

edited