Add `unchecked_disjoint_bitor` per ACP373 #135760

scottmcm · 2025-01-20T08:04:42Z

Following the names from libs-api in rust-lang/libs-team#373 (comment)

Includes a fallback implementation so this doesn't have to update cg_clif or cg_gcc, and overrides it in cg_llvm to use or disjoint, which is available in LLVM 18 so hopefully we don't need any version checks.

rustbot · 2025-01-20T08:04:50Z

r? @Amanieu

rustbot has assigned @Amanieu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2025-01-20T08:04:52Z

Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter
gets adapted for the changes, if necessary.

cc @rust-lang/miri, @rust-lang/wg-const-eval

library/core/src/num/uint_macros.rs

WaffleLapkin · 2025-01-20T10:24:19Z

compiler/rustc_codegen_ssa/src/traits/builder.rs

    fn or(&mut self, lhs: Self::Value, rhs: Self::Value) -> Self::Value;
+    /// Defaults to [`Self::or`], but guarantees `(lhs & rhs) == 0` so some backends
+    /// can emit something more helpful for optimizations.
+    fn or_disjoint(&mut self, lhs: Self::Value, rhs: Self::Value) -> Self::Value {


Why is this a method and not just a normal intrinsic? Why does this have 2 default implementations? (fallback in core and default impl here)

So that other places in cg_ssa can use it if it's helpful.

The fallback one in core is for clif and ctfe, whereas the one here is for gcc.

Kinda annoying that we need 2 fallbacks :(

Is this what we do for all intrinsics that can't be implemented in cg_ssa? Isn't there some place where cg_llvm matches on the intrinsic name to provide its own impls before falling back to the cg_ssa one?

I could do that, it just feels like a worse fit here.

This isn't like these intrinsics

rust/compiler/rustc_codegen_llvm/src/intrinsic.rs

Lines 29 to 154 in a30f915

fn get_simple_intrinsic<'ll>(

cx: &CodegenCx<'ll, '_>,

name: Symbol,

) -> Option<(&'ll Type, &'ll Value)> {

let llvm_name = match name {

sym::sqrtf16 => "llvm.sqrt.f16",

sym::sqrtf32 => "llvm.sqrt.f32",

sym::sqrtf64 => "llvm.sqrt.f64",

sym::sqrtf128 => "llvm.sqrt.f128",

sym::powif16 => "llvm.powi.f16.i32",

sym::powif32 => "llvm.powi.f32.i32",

sym::powif64 => "llvm.powi.f64.i32",

sym::powif128 => "llvm.powi.f128.i32",

sym::sinf16 => "llvm.sin.f16",

sym::sinf32 => "llvm.sin.f32",

sym::sinf64 => "llvm.sin.f64",

sym::sinf128 => "llvm.sin.f128",

sym::cosf16 => "llvm.cos.f16",

sym::cosf32 => "llvm.cos.f32",

sym::cosf64 => "llvm.cos.f64",

sym::cosf128 => "llvm.cos.f128",

sym::powf16 => "llvm.pow.f16",

sym::powf32 => "llvm.pow.f32",

sym::powf64 => "llvm.pow.f64",

sym::powf128 => "llvm.pow.f128",

sym::expf16 => "llvm.exp.f16",

sym::expf32 => "llvm.exp.f32",

sym::expf64 => "llvm.exp.f64",

sym::expf128 => "llvm.exp.f128",

sym::exp2f16 => "llvm.exp2.f16",

sym::exp2f32 => "llvm.exp2.f32",

sym::exp2f64 => "llvm.exp2.f64",

sym::exp2f128 => "llvm.exp2.f128",

sym::logf16 => "llvm.log.f16",

sym::logf32 => "llvm.log.f32",

sym::logf64 => "llvm.log.f64",

sym::logf128 => "llvm.log.f128",

sym::log10f16 => "llvm.log10.f16",

sym::log10f32 => "llvm.log10.f32",

sym::log10f64 => "llvm.log10.f64",

sym::log10f128 => "llvm.log10.f128",

sym::log2f16 => "llvm.log2.f16",

sym::log2f32 => "llvm.log2.f32",

sym::log2f64 => "llvm.log2.f64",

sym::log2f128 => "llvm.log2.f128",

sym::fmaf16 => "llvm.fma.f16",

sym::fmaf32 => "llvm.fma.f32",

sym::fmaf64 => "llvm.fma.f64",

sym::fmaf128 => "llvm.fma.f128",

sym::fmuladdf16 => "llvm.fmuladd.f16",

sym::fmuladdf32 => "llvm.fmuladd.f32",

sym::fmuladdf64 => "llvm.fmuladd.f64",

sym::fmuladdf128 => "llvm.fmuladd.f128",

sym::fabsf16 => "llvm.fabs.f16",

sym::fabsf32 => "llvm.fabs.f32",

sym::fabsf64 => "llvm.fabs.f64",

sym::fabsf128 => "llvm.fabs.f128",

sym::minnumf16 => "llvm.minnum.f16",

sym::minnumf32 => "llvm.minnum.f32",

sym::minnumf64 => "llvm.minnum.f64",

sym::minnumf128 => "llvm.minnum.f128",

sym::maxnumf16 => "llvm.maxnum.f16",

sym::maxnumf32 => "llvm.maxnum.f32",

sym::maxnumf64 => "llvm.maxnum.f64",

sym::maxnumf128 => "llvm.maxnum.f128",

sym::copysignf16 => "llvm.copysign.f16",

sym::copysignf32 => "llvm.copysign.f32",

sym::copysignf64 => "llvm.copysign.f64",

sym::copysignf128 => "llvm.copysign.f128",

sym::floorf16 => "llvm.floor.f16",

sym::floorf32 => "llvm.floor.f32",

sym::floorf64 => "llvm.floor.f64",

sym::floorf128 => "llvm.floor.f128",

sym::ceilf16 => "llvm.ceil.f16",

sym::ceilf32 => "llvm.ceil.f32",

sym::ceilf64 => "llvm.ceil.f64",

sym::ceilf128 => "llvm.ceil.f128",

sym::truncf16 => "llvm.trunc.f16",

sym::truncf32 => "llvm.trunc.f32",

sym::truncf64 => "llvm.trunc.f64",

sym::truncf128 => "llvm.trunc.f128",

sym::rintf16 => "llvm.rint.f16",

sym::rintf32 => "llvm.rint.f32",

sym::rintf64 => "llvm.rint.f64",

sym::rintf128 => "llvm.rint.f128",

sym::nearbyintf16 => "llvm.nearbyint.f16",

sym::nearbyintf32 => "llvm.nearbyint.f32",

sym::nearbyintf64 => "llvm.nearbyint.f64",

sym::nearbyintf128 => "llvm.nearbyint.f128",

sym::roundf16 => "llvm.round.f16",

sym::roundf32 => "llvm.round.f32",

sym::roundf64 => "llvm.round.f64",

sym::roundf128 => "llvm.round.f128",

sym::ptr_mask => "llvm.ptrmask",

sym::roundevenf16 => "llvm.roundeven.f16",

sym::roundevenf32 => "llvm.roundeven.f32",

sym::roundevenf64 => "llvm.roundeven.f64",

sym::roundevenf128 => "llvm.roundeven.f128",

_ => return None,

};

Some(cx.get_intrinsic(llvm_name))

}

where we're lowering it to some call, or the ones depending on LLVM-only integer types, or whatever.

It's far more like add nuw, where we have add, unchecked_sadd, and unchecked_uadd on BuilderMethods, because it's an instruction in the IR and something that we can use in other places in MIR-to-Backend lowering. (For example, we could use it in emitting rotates since we might as well and it's no less safe than the shifts that are also unchecked in the builder.)

And sure, those two unchecked examples don't have defaults today, but they should, because cg_gcc is just emitting self.gcc_add(a, b) for all three anyway, and if we want cg_ssa to be useful it shouldn't require implementing these when there's perfectly fine -- albeit potentially suboptimal -- provided implementations.

library/core/src/intrinsics/mod.rs

RalfJung

Could you add a fail test in Miri? Just to be sure someone doesn't remove the assume at some point.

library/core/src/intrinsics/fallback.rs

library/core/src/num/uint_macros.rs

rustbot · 2025-01-22T09:04:31Z

The Miri subtree was changed

cc @rust-lang/miri

scottmcm · 2025-01-22T09:05:11Z

Added some miri-conditional track_callers to get the miri output nice.

library/core/src/intrinsics/mod.rs

library/core/src/num/uint_macros.rs

library/core/src/intrinsics/fallback.rs

WaffleLapkin · 2025-02-01T08:25:42Z

@bors r+

bors · 2025-02-01T08:25:44Z

📌 Commit 5e6ae8b has been approved by WaffleLapkin

It is now in the queue for this repository.

…apkin Add `unchecked_disjoint_bitor` per ACP373 Following the names from libs-api in rust-lang/libs-team#373 (comment) Includes a fallback implementation so this doesn't have to update cg_clif or cg_gcc, and overrides it in cg_llvm to use `or disjoint`, which [is available in LLVM 18](https://releases.llvm.org/18.1.0/docs/LangRef.html#or-instruction) so hopefully we don't need any version checks.

…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#130514 (Implement MIR lowering for unsafe binders) - rust-lang#135684 (docs: Documented Send and Sync requirements for Mutex + MutexGuard) - rust-lang#135760 (Add `unchecked_disjoint_bitor` per ACP373) - rust-lang#136154 (Use +secure-plt for powerpc-unknown-linux-gnu{,spe}) - rust-lang#136309 (set rustc dylib on manually constructed rustc command) - rust-lang#136339 (CompileTest: Add Directives to Ignore `arm-unknown-*` Targets) - rust-lang#136368 (Make comma separated lists of anything easier to make for errors) r? `@ghost` `@rustbot` modify labels: rollup

matthiaskrgr · 2025-02-01T15:39:00Z

@bors r-
this probably failed here?
#136388 (comment)

saethlin · 2025-02-02T20:00:32Z

@bors try

saethlin · 2025-02-02T20:01:12Z

@bors r-

Add `unchecked_disjoint_bitor` per ACP373 Following the names from libs-api in rust-lang/libs-team#373 (comment) Includes a fallback implementation so this doesn't have to update cg_clif or cg_gcc, and overrides it in cg_llvm to use `or disjoint`, which [is available in LLVM 18](https://releases.llvm.org/18.1.0/docs/LangRef.html#or-instruction) so hopefully we don't need any version checks. try-job: x86_64-gnu-distcheck

bors · 2025-02-02T20:01:46Z

⌛ Trying commit 5e6ae8b with merge 20e70db...

bors · 2025-02-02T20:56:30Z

💔 Test failed - checks-actions

rustbot assigned Amanieu Jan 20, 2025

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. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jan 20, 2025

scottmcm mentioned this pull request Jan 20, 2025

Tracking Issue for unchecked_disjoint_bitor #135758

Open

3 tasks

TDecking reviewed Jan 20, 2025

View reviewed changes

library/core/src/num/uint_macros.rs Show resolved Hide resolved

WaffleLapkin reviewed Jan 20, 2025

View reviewed changes

RalfJung reviewed Jan 20, 2025

View reviewed changes

library/core/src/intrinsics/mod.rs Outdated Show resolved Hide resolved

RalfJung reviewed Jan 20, 2025

View reviewed changes

library/core/src/intrinsics/fallback.rs Show resolved Hide resolved

Sky9x reviewed Jan 22, 2025

View reviewed changes

library/core/src/num/uint_macros.rs Outdated Show resolved Hide resolved

WaffleLapkin reviewed Jan 22, 2025

View reviewed changes

library/core/src/intrinsics/mod.rs Outdated Show resolved Hide resolved

WaffleLapkin reviewed Jan 22, 2025

View reviewed changes

library/core/src/num/uint_macros.rs Outdated Show resolved Hide resolved

library/core/src/num/uint_macros.rs Outdated Show resolved Hide resolved

library/core/src/intrinsics/fallback.rs Show resolved Hide resolved

scottmcm added 4 commits January 31, 2025 22:29

Add unchecked_disjoint_bitor with fallback intrinsic implementation

f230253

Override disjoint_or in the LLVM backend

4ee1602

PR feedback

61150a8

More PR feedback

5e6ae8b

scottmcm force-pushed the disjoint-bitor branch from ae46ade to 5e6ae8b Compare February 1, 2025 06:31

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 Feb 1, 2025

matthiaskrgr mentioned this pull request Feb 1, 2025

Rollup of 7 pull requests #136388

Closed

bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Feb 1, 2025

This comment has been minimized.

Sign in to view

	fn get_simple_intrinsic<'ll>(
	cx: &CodegenCx<'ll, '_>,
	name: Symbol,
	) -> Option<(&'ll Type, &'ll Value)> {
	let llvm_name = match name {
	sym::sqrtf16 => "llvm.sqrt.f16",
	sym::sqrtf32 => "llvm.sqrt.f32",
	sym::sqrtf64 => "llvm.sqrt.f64",
	sym::sqrtf128 => "llvm.sqrt.f128",

	sym::powif16 => "llvm.powi.f16.i32",
	sym::powif32 => "llvm.powi.f32.i32",
	sym::powif64 => "llvm.powi.f64.i32",
	sym::powif128 => "llvm.powi.f128.i32",

	sym::sinf16 => "llvm.sin.f16",
	sym::sinf32 => "llvm.sin.f32",
	sym::sinf64 => "llvm.sin.f64",
	sym::sinf128 => "llvm.sin.f128",

	sym::cosf16 => "llvm.cos.f16",
	sym::cosf32 => "llvm.cos.f32",
	sym::cosf64 => "llvm.cos.f64",
	sym::cosf128 => "llvm.cos.f128",

	sym::powf16 => "llvm.pow.f16",
	sym::powf32 => "llvm.pow.f32",
	sym::powf64 => "llvm.pow.f64",
	sym::powf128 => "llvm.pow.f128",

	sym::expf16 => "llvm.exp.f16",
	sym::expf32 => "llvm.exp.f32",
	sym::expf64 => "llvm.exp.f64",
	sym::expf128 => "llvm.exp.f128",

	sym::exp2f16 => "llvm.exp2.f16",
	sym::exp2f32 => "llvm.exp2.f32",
	sym::exp2f64 => "llvm.exp2.f64",
	sym::exp2f128 => "llvm.exp2.f128",

	sym::logf16 => "llvm.log.f16",
	sym::logf32 => "llvm.log.f32",
	sym::logf64 => "llvm.log.f64",
	sym::logf128 => "llvm.log.f128",

	sym::log10f16 => "llvm.log10.f16",
	sym::log10f32 => "llvm.log10.f32",
	sym::log10f64 => "llvm.log10.f64",
	sym::log10f128 => "llvm.log10.f128",

	sym::log2f16 => "llvm.log2.f16",
	sym::log2f32 => "llvm.log2.f32",
	sym::log2f64 => "llvm.log2.f64",
	sym::log2f128 => "llvm.log2.f128",

	sym::fmaf16 => "llvm.fma.f16",
	sym::fmaf32 => "llvm.fma.f32",
	sym::fmaf64 => "llvm.fma.f64",
	sym::fmaf128 => "llvm.fma.f128",

	sym::fmuladdf16 => "llvm.fmuladd.f16",
	sym::fmuladdf32 => "llvm.fmuladd.f32",
	sym::fmuladdf64 => "llvm.fmuladd.f64",
	sym::fmuladdf128 => "llvm.fmuladd.f128",

	sym::fabsf16 => "llvm.fabs.f16",
	sym::fabsf32 => "llvm.fabs.f32",
	sym::fabsf64 => "llvm.fabs.f64",
	sym::fabsf128 => "llvm.fabs.f128",

	sym::minnumf16 => "llvm.minnum.f16",
	sym::minnumf32 => "llvm.minnum.f32",
	sym::minnumf64 => "llvm.minnum.f64",
	sym::minnumf128 => "llvm.minnum.f128",

	sym::maxnumf16 => "llvm.maxnum.f16",
	sym::maxnumf32 => "llvm.maxnum.f32",
	sym::maxnumf64 => "llvm.maxnum.f64",
	sym::maxnumf128 => "llvm.maxnum.f128",

	sym::copysignf16 => "llvm.copysign.f16",
	sym::copysignf32 => "llvm.copysign.f32",
	sym::copysignf64 => "llvm.copysign.f64",
	sym::copysignf128 => "llvm.copysign.f128",

	sym::floorf16 => "llvm.floor.f16",
	sym::floorf32 => "llvm.floor.f32",
	sym::floorf64 => "llvm.floor.f64",
	sym::floorf128 => "llvm.floor.f128",

	sym::ceilf16 => "llvm.ceil.f16",
	sym::ceilf32 => "llvm.ceil.f32",
	sym::ceilf64 => "llvm.ceil.f64",
	sym::ceilf128 => "llvm.ceil.f128",

	sym::truncf16 => "llvm.trunc.f16",
	sym::truncf32 => "llvm.trunc.f32",
	sym::truncf64 => "llvm.trunc.f64",
	sym::truncf128 => "llvm.trunc.f128",

	sym::rintf16 => "llvm.rint.f16",
	sym::rintf32 => "llvm.rint.f32",
	sym::rintf64 => "llvm.rint.f64",
	sym::rintf128 => "llvm.rint.f128",

	sym::nearbyintf16 => "llvm.nearbyint.f16",
	sym::nearbyintf32 => "llvm.nearbyint.f32",
	sym::nearbyintf64 => "llvm.nearbyint.f64",
	sym::nearbyintf128 => "llvm.nearbyint.f128",

	sym::roundf16 => "llvm.round.f16",
	sym::roundf32 => "llvm.round.f32",
	sym::roundf64 => "llvm.round.f64",
	sym::roundf128 => "llvm.round.f128",

	sym::ptr_mask => "llvm.ptrmask",

	sym::roundevenf16 => "llvm.roundeven.f16",
	sym::roundevenf32 => "llvm.roundeven.f32",
	sym::roundevenf64 => "llvm.roundeven.f64",
	sym::roundevenf128 => "llvm.roundeven.f128",

	_ => return None,
	};
	Some(cx.get_intrinsic(llvm_name))
	}

Uh oh!

Add unchecked_disjoint_bitor per ACP373 #135760

Add unchecked_disjoint_bitor per ACP373 #135760

Uh oh!

Conversation

scottmcm commented Jan 20, 2025 • edited by saethlin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jan 20, 2025

Uh oh!

rustbot commented Jan 20, 2025

Uh oh!

Uh oh!

WaffleLapkin Jan 20, 2025

Choose a reason for hiding this comment

Uh oh!

scottmcm Jan 22, 2025

Choose a reason for hiding this comment

Uh oh!

WaffleLapkin Jan 22, 2025

Choose a reason for hiding this comment

Uh oh!

RalfJung Jan 22, 2025

Choose a reason for hiding this comment

Uh oh!

scottmcm Jan 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

RalfJung left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rustbot commented Jan 22, 2025

Uh oh!

scottmcm commented Jan 22, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

WaffleLapkin commented Feb 1, 2025

Uh oh!

bors commented Feb 1, 2025

Uh oh!

matthiaskrgr commented Feb 1, 2025

Uh oh!

saethlin commented Feb 2, 2025

Uh oh!

saethlin commented Feb 2, 2025

Uh oh!

bors commented Feb 2, 2025

Uh oh!

This comment has been minimized.

bors commented Feb 2, 2025

Uh oh!

Add `unchecked_disjoint_bitor` per ACP373 #135760

Add `unchecked_disjoint_bitor` per ACP373 #135760

scottmcm commented Jan 20, 2025 •

edited by saethlin

Loading