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

@@ -167,6 +167,11 @@ pub trait BuilderMethods<'a, 'tcx>:
    fn unchecked_umul(&mut self, lhs: Self::Value, rhs: Self::Value) -> Self::Value;
    fn and(&mut self, lhs: Self::Value, rhs: Self::Value) -> Self::Value;
    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-

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

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

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

💔 Test failed - checks-actions

saethlin · 2025-02-02T20:56:38Z

The problem was this LLVM assertion:

2025-02-02T20:54:51.7744534Z rustc: /checkout/src/llvm-project/llvm/include/llvm/Support/Casting.h:578: decltype(auto) llvm::cast(From*) [with To = llvm::PossiblyDisjointInst; From = llvm::Value]: Assertion `isa<To>(Val) && "cast<Ty>() argument of incompatible type!"' failed.

scottmcm · 2025-02-03T04:57:44Z

Oh, does CI not have assertions enabled? b987aa5 triggered it locally for me.

scottmcm · 2025-02-03T05:11:00Z

Turns out that LLVMBuildOr doesn't always return an or Instruction, so added a check in f46e6be?diff=unified&w=1

You can see that the upstream LLVMBuildNUWNeg also needs a check to ensure there's an instruction

https://github.com/llvm/llvm-project/blob/31db7afacf4dae051fcd0da22e440813663b61f3/llvm/lib/IR/Core.cpp#L3762-L3768

scottmcm · 2025-02-03T06:10:20Z

@bors r=WaffleLapkin rollup=iffy (failed in rollup last time, but I repro'd it locally and fixed)

bors · 2025-02-03T06:10:23Z

📌 Commit f46e6be has been approved by WaffleLapkin

It is now in the queue for this repository.

bors · 2025-02-04T17:46:10Z

⌛ Testing commit f46e6be with merge 3f33b30...

bors · 2025-02-04T20:54:24Z

☀️ Test successful - checks-actions
Approved by: WaffleLapkin
Pushing 3f33b30 to master...

rust-timer · 2025-02-04T22:12:10Z

Finished benchmarking commit (3f33b30): 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 (primary 1.7%, secondary 2.6%)

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.7%	[1.7%, 1.7%]	1
Regressions ❌ (secondary)	2.6%	[1.0%, 4.3%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.7%	[1.7%, 1.7%]	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: 778.094s -> 778.717s (0.08%)
Artifact size: 328.84 MiB -> 328.88 MiB (0.01%)

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

scottmcm reopened this Feb 1, 2025

This comment has been minimized.

Sign in to view

Handle the case where the or disjoint folds immediately to a constant

f46e6be

scottmcm force-pushed the disjoint-bitor branch from b987aa5 to f46e6be Compare February 3, 2025 05:04

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 3, 2025

scottmcm mentioned this pull request Feb 3, 2025

Add #[optimize(none)] #128657

Merged

bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 4, 2025

bors merged commit 3f33b30 into rust-lang:master Feb 4, 2025
7 checks passed

rustbot added this to the 1.86.0 milestone Feb 4, 2025

bors mentioned this pull request Feb 4, 2025

Add (checked_)norem_div methods for integer types #116632

Closed

scottmcm deleted the disjoint-bitor branch February 5, 2025 11:16

	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))
	}

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

saethlin commented Feb 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

scottmcm commented Feb 3, 2025

Uh oh!

scottmcm commented Feb 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

scottmcm commented Feb 3, 2025

Uh oh!

bors commented Feb 3, 2025

Uh oh!

bors commented Feb 4, 2025

Uh oh!

bors commented Feb 4, 2025

Uh oh!

Uh oh!

rust-timer commented Feb 4, 2025

Overall result: no relevant changes - no action needed

Uh oh!

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

saethlin commented Feb 2, 2025 •

edited

Loading

scottmcm commented Feb 3, 2025 •

edited

Loading