Use `LLVMIntrinsicGetDeclaration` to completely remove the hardcoded intrinsics list #142521

sayantn · 2025-06-14T22:35:46Z

Follow-up to #142259

This also needs a rustc-perf run, because Intrinsic::getType can be expensive

@rustbot label A-LLVM A-codegen T-compiler
r? @workingjubilee
cc @nikic

workingjubilee · 2025-06-14T22:39:44Z

@bors2 try @rust-timer queue

Use `LLVMIntrinsicGetDeclaration` to completely remove the hardcoded intrinsics list Follow-up to #142259 This also needs a rustc-perf run, because `Intrinsic::getType` can be expensive `@rustbot` label A-LLVM A-codegen T-compiler r? `@workingjubilee` cc `@nikic`

rust-bors · 2025-06-14T22:39:49Z

⌛ Trying commit 10a73c9 with merge 5e963b3…

To cancel the try build, run the command @bors2 try cancel.

workingjubilee · 2025-06-14T22:51:55Z

hum.

rust-bors · 2025-06-14T23:07:23Z

💔 Test failed

CI ❌

sayantn · 2025-06-14T23:15:49Z

interesting segfault, I have a suspicion that this is due to get_type_of_global. Let me try without that

sayantn · 2025-06-15T00:00:05Z

ah, so it was due to a mistake in the last PR, it didn't have any type parameters for llvm.threadlocal.address, where it should have 1 (surprisingly LLVMCopyOverloadedName2 didn't complain about this)

workingjubilee · 2025-06-15T00:30:29Z

Oh, distressing.

workingjubilee · 2025-06-15T00:33:32Z

@bors2 try

rust-bors · 2025-06-15T00:33:35Z

⌛ Trying commit 7504607 with merge 904f591…

To cancel the try build, run the command @bors2 try cancel.

Use `LLVMIntrinsicGetDeclaration` to completely remove the hardcoded intrinsics list Follow-up to #142259 This also needs a rustc-perf run, because `Intrinsic::getType` can be expensive `@rustbot` label A-LLVM A-codegen T-compiler r? `@workingjubilee` cc `@nikic`

bors · 2025-06-15T00:33:45Z

⌛ Trying commit 7504607 with merge f92eca7...

sayantn · 2025-06-15T00:33:58Z

is bors ghosting us now?

workingjubilee · 2025-06-15T00:34:00Z

oh no, the latency.

workingjubilee · 2025-06-15T00:34:22Z

@bors2 try cancel

rust-bors · 2025-06-15T00:34:24Z

Try build cancelled. Cancelled workflows:

https://github.com/rust-lang/rust/actions/runs/15657618753

workingjubilee · 2025-06-15T00:34:47Z

AHA, MY PLAN WAS TO TEST THE CANCEL FUNCTION ALL ALONG

sayantn · 2025-06-15T00:35:34Z

that is some serious difference in latency.

bors · 2025-06-15T03:01:04Z

☀️ Try build successful - checks-actions
Build commit: f92eca7 (f92eca7e3359f7ee0ba466a9b35495d947e2d602)

rust-timer · 2025-06-15T05:18:41Z

Finished benchmarking commit (f92eca7): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.4%	[-1.4%, -1.4%]	1
Improvements ✅ (secondary)	-1.3%	[-1.3%, -1.3%]	1
All ❌✅ (primary)	-1.4%	[-1.4%, -1.4%]	1

Max RSS (memory usage)

Results (secondary 0.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.5%	[2.5%, 2.5%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.0%	[-2.0%, -2.0%]	1
All ❌✅ (primary)	-	-	0

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: 758.463s -> 757.066s (-0.18%)
Artifact size: 372.25 MiB -> 372.23 MiB (-0.00%)

sayantn · 2025-06-15T05:47:30Z

Yay! No perf regression ❤️

compiler/rustc_codegen_llvm/src/context.rs

compiler/rustc_codegen_llvm/src/builder.rs

nikic · 2025-06-15T10:41:04Z

compiler/rustc_codegen_llvm/src/context.rs

@@ -138,7 +138,7 @@ pub(crate) struct FullCx<'ll, 'tcx> {
    pub rust_try_fn: Cell<Option<(&'ll Type, &'ll Value)>>,

    intrinsics:
-        RefCell<FxHashMap<(&'static str, SmallVec<[&'ll Type; 2]>), (&'ll Type, &'ll Value)>>,
+        RefCell<FxHashMap<(Cow<'static, str>, SmallVec<[&'ll Type; 2]>), (&'ll Type, &'ll Value)>>,


Did this become a Cow because previously everything was indirected via the &'static strs in declare_intrinsic, and now no longer is?

I just wanted to reduce some allocations (because most intrinsic names are string literals, but some are generated by format!). String would work as well. Should I change it?

It should be fine.

…intrinsics list

nikic

Looks good to me.

sayantn · 2025-06-15T17:03:14Z

There is one more place where we should use val_ty instead of type_ptr - in va_copy. But there the LLVM signature is (0, 0) -> void, and the Rust signature is like (*mut T, &T) -> void. Is it guaranteed that *mut T and &T have the same LLVM address space (T is Sized)? Otherwise we might need to add some addressspacecasts to make it sound.

nikic · 2025-06-15T17:08:27Z

There is one more place where we should use val_ty instead of type_ptr - in va_copy. But there the LLVM signature is (0, 0) -> void, and the Rust signature is like (*mut T, &T) -> void. Is it guaranteed that *mut T and &T have the same LLVM address space (T is Sized)? Otherwise we might need to add some addressspacecasts to make it sound.

Yes, I believe this is guaranteed.

sayantn · 2025-06-15T17:11:25Z

Nice, then I am changing that too

edit: I mean using val_ty instead of type_ptr

…cations

workingjubilee · 2025-06-15T18:34:03Z

@bors r+

bors · 2025-06-15T18:34:06Z

📌 Commit a9500d6 has been approved by workingjubilee

It is now in the queue for this repository.

workingjubilee · 2025-06-15T18:34:13Z

@bors r=nikic,workingjubilee

bors · 2025-06-15T18:34:15Z

💡 This pull request was already approved, no need to approve it again.

There's another pull request that is currently being tested, blocking this pull request: collect delayed lints in hir_crate_items #142455

bors · 2025-06-15T18:34:16Z

📌 Commit a9500d6 has been approved by nikic,workingjubilee

It is now in the queue for this repository.

bors · 2025-06-16T03:40:21Z

⌛ Testing commit a9500d6 with merge 68ac5ab...

bors · 2025-06-16T06:57:05Z

☀️ Test successful - checks-actions
Approved by: nikic,workingjubilee
Pushing 68ac5ab to master...

github-actions · 2025-06-16T07:00:22Z

What is this?

This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing e314b97 (parent) -> 68ac5ab (this PR)

Test differences

No test diffs found

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard 68ac5abb067806a88464ddbfbd3c7eec877b488d --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

x86_64-apple-2: 4492.8s -> 3437.3s (-23.5%)
dist-x86_64-apple: 9719.0s -> 11073.5s (13.9%)
i686-gnu-2: 6098.5s -> 5398.0s (-11.5%)
mingw-check-1: 1877.1s -> 1685.1s (-10.2%)
x86_64-apple-1: 7080.0s -> 6425.2s (-9.2%)
x86_64-rust-for-linux: 2831.3s -> 2578.8s (-8.9%)
x86_64-gnu-aux: 6486.8s -> 5916.8s (-8.8%)
i686-gnu-1: 8029.5s -> 7324.0s (-8.8%)
i686-gnu-nopt-1: 8000.6s -> 7313.0s (-8.6%)
arm-android: 5946.2s -> 5473.2s (-8.0%)

How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

rust-timer · 2025-06-16T08:25:08Z

Finished benchmarking commit (68ac5ab): comparison URL.

Overall result: ❌✅ regressions and improvements - no action needed

@rustbot label: -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	1.1%	[1.1%, 1.1%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.7%	[-0.7%, -0.7%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

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

Cycles

Results (primary 2.2%, secondary -9.4%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	2.2%	[2.2%, 2.2%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-9.4%	[-9.4%, -9.4%]	1
All ❌✅ (primary)	2.2%	[2.2%, 2.2%]	1

Binary size

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

Bootstrap: 756.881s -> 757.272s (0.05%)
Artifact size: 372.07 MiB -> 372.17 MiB (0.03%)

rustbot assigned workingjubilee Jun 14, 2025

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 14, 2025

This comment has been minimized.

Sign in to view

sayantn force-pushed the simplify-intrinsics branch from 10a73c9 to 7504607 Compare June 15, 2025 00:00

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 15, 2025

klensy reviewed Jun 15, 2025

View reviewed changes

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

nikic reviewed Jun 15, 2025

View reviewed changes

Use LLVMIntrinsicGetDeclaration to completely remove the hardcoded …

9415f3d

…intrinsics list

sayantn force-pushed the simplify-intrinsics branch from 7504607 to 3dc8ebc Compare June 15, 2025 16:45

nikic approved these changes Jun 15, 2025

View reviewed changes

Correctly account for different address spaces in LLVM intrinsic invo…

a9500d6

…cations

sayantn force-pushed the simplify-intrinsics branch from 3dc8ebc to a9500d6 Compare June 15, 2025 17:19

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 Jun 15, 2025

bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 16, 2025

bors merged commit 68ac5ab into rust-lang:master Jun 16, 2025
11 checks passed

rustbot added this to the 1.89.0 milestone Jun 16, 2025

bors mentioned this pull request Jun 16, 2025

Change codegen of LLVM intrinsics to be name-based, and add llvm linkage support for bf16(xN), i1xN and x86amx #140763

Open

Use LLVMIntrinsicGetDeclaration to completely remove the hardcoded intrinsics list #142521

Use LLVMIntrinsicGetDeclaration to completely remove the hardcoded intrinsics list #142521

Uh oh!

Conversation

sayantn commented Jun 14, 2025 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

workingjubilee commented Jun 14, 2025

Uh oh!

This comment has been minimized.

rust-bors bot commented Jun 14, 2025

Uh oh!

This comment has been minimized.

workingjubilee commented Jun 14, 2025

Uh oh!

rust-bors bot commented Jun 14, 2025

Uh oh!

sayantn commented Jun 14, 2025

Uh oh!

sayantn commented Jun 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

workingjubilee commented Jun 15, 2025

Uh oh!

workingjubilee commented Jun 15, 2025

Uh oh!

rust-bors bot commented Jun 15, 2025

Uh oh!

bors commented Jun 15, 2025

Uh oh!

sayantn commented Jun 15, 2025

Uh oh!

workingjubilee commented Jun 15, 2025

Uh oh!

workingjubilee commented Jun 15, 2025

Uh oh!

rust-bors bot commented Jun 15, 2025

Uh oh!

workingjubilee commented Jun 15, 2025

Uh oh!

sayantn commented Jun 15, 2025

Uh oh!

bors commented Jun 15, 2025

Uh oh!

This comment has been minimized.

rust-timer commented Jun 15, 2025

Overall result: ✅ improvements - no action needed

Uh oh!

sayantn commented Jun 15, 2025

Uh oh!

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!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

sayantn commented Jun 15, 2025

Uh oh!

nikic commented Jun 15, 2025

Uh oh!

sayantn commented Jun 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

workingjubilee commented Jun 15, 2025

Uh oh!

bors commented Jun 15, 2025

Uh oh!

workingjubilee commented Jun 15, 2025

Uh oh!

bors commented Jun 15, 2025

Uh oh!

bors commented Jun 15, 2025

Uh oh!

bors commented Jun 16, 2025

Uh oh!

Use `LLVMIntrinsicGetDeclaration` to completely remove the hardcoded intrinsics list #142521

Use `LLVMIntrinsicGetDeclaration` to completely remove the hardcoded intrinsics list #142521

sayantn commented Jun 14, 2025 •

edited by rustbot

Loading

sayantn commented Jun 15, 2025 •

edited

Loading

sayantn commented Jun 15, 2025 •

edited

Loading