Skip to content

llvm: use intrinsics for f16, f32 minimum/maximum#158330

Merged
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
RalfJung:minimum-intrinsic
Jun 26, 2026
Merged

llvm: use intrinsics for f16, f32 minimum/maximum#158330
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
RalfJung:minimum-intrinsic

Conversation

@RalfJung

@RalfJung RalfJung commented Jun 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

The mentioned LLVM issues seem to only affect f64 and f128.

Cc @nikic @Urgau @tgross35

@rustbot rustbot added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. 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 Jun 23, 2026
@rustbot

rustbot commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

r? @nnethercote

rustbot has assigned @nnethercote.
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

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 73 candidates
  • Random selection from 20 candidates

tgross35
tgross35 approved these changes Jun 23, 2026
View reviewed changes

@tgross35 tgross35 left a comment •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this makes sense to at least try, especially given #141087 has been resolved. But Nikita probably knows the status better.

r? nikic

View changes since this review

@rustbot rustbot assigned nikic and unassigned nnethercote Jun 23, 2026
@rust-log-analyzer

This comment was marked as outdated.

Sign in to view
@RalfJung

RalfJung commented Jun 24, 2026

Copy link
Copy Markdown
Member Author

llvm/llvm-project#140193 has landed in the mean time so I wonder if we can actually enable this intrinsic even for f64 now?

@RalfJung

RalfJung commented Jun 24, 2026

Copy link
Copy Markdown
Member Author

@bors try

@rust-bors

This comment has been minimized.

Sign in to view
rust-bors Bot pushed a commit that referenced this pull request Jun 24, 2026
@bors
Auto merge of #158330 - RalfJung:minimum-intrinsic, r=<try>
a6fd13e 
llvm: use intrinsics for f16, f32 minimum/maximum

try-job: dist-i586\*
try-job: dist-arm\*
try-job: arm\*
@rust-bors rust-bors Bot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 24, 2026
@rust-bors

rust-bors Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

💔 Test for a6fd13e failed: CI. Failed job:

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@nikic

nikic commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

llvm/llvm-project#140193 has landed in the mean time so I wonder if we can actually enable this intrinsic even for f64 now?

This is for minimumnum, not minimum. I believe minimum is still broken.

@RalfJung

RalfJung commented Jun 24, 2026

Copy link
Copy Markdown
Member Author

Ah I see. I'll remove the 2nd commit then.

Also apparently try-jobs has a different syntax than "try jobs=..."...

@bors try jobs=dist-i586*,dist-arm*,arm*

@rust-bors

This comment has been minimized.

Sign in to view
rust-bors Bot pushed a commit that referenced this pull request Jun 24, 2026
@bors
Auto merge of #158330 - RalfJung:minimum-intrinsic, r=<try>
b3aac6f 
llvm: use intrinsics for f16, f32 minimum/maximum


try-job: dist-i586*
try-job: dist-arm*
try-job: arm*
@RalfJung RalfJung force-pushed the minimum-intrinsic branch from fbab069 to bd757a6 Compare June 24, 2026 10:04
@RalfJung

RalfJung commented Jun 24, 2026

Copy link
Copy Markdown
Member Author

@bors try jobs=dist-i586*,dist-arm*,arm*

@rust-bors

This comment has been minimized.

Sign in to view
rust-bors Bot pushed a commit that referenced this pull request Jun 24, 2026
@bors
Auto merge of #158330 - RalfJung:minimum-intrinsic, r=<try>
d462de2 
llvm: use intrinsics for f16, f32 minimum/maximum


try-job: dist-i586*
try-job: dist-arm*
try-job: arm*
@rust-bors

rust-bors Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: d462de2 (d462de27637d0e1b2939faca516d6ccb51c0dc24)
Base parent: f28ac76 (f28ac764c36004fa6a6e098d15b4016a838c13c6)

@RalfJung

RalfJung commented Jun 24, 2026

Copy link
Copy Markdown
Member Author

@rustbot ready

@nikic this part should hopefully be good then.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 24, 2026
@nikic

nikic commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

What's the motivation for using the intrinsics for some types? I'm not opposed, but also not clear we should do this given that the underlying issues are still open.

@RalfJung

RalfJung commented Jun 24, 2026

Copy link
Copy Markdown
Member Author

There are no open issues I am aware of for f16/f32. So if we use them and issues pop up that still seems useful, then we'll file more LLVM bugs. IOW, we should test-drive the parts of the intrinsics that we think are working correctly.

@nikic

nikic commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

@bors r+

@rust-bors

rust-bors Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

📌 Commit bd757a6 has been approved by nikic

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 4. This pull request will be tested once the tree is reopened.

@rust-bors rust-bors Bot 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 25, 2026
@jhpratt jhpratt mentioned this pull request Jun 25, 2026
Merged
rust-bors Bot pushed a commit that referenced this pull request Jun 25, 2026
@bors
Auto merge of #158419 - jhpratt:rollup-38sJqWx, r=jhpratt
40557f6 
Rollup of 10 pull requests

Successful merges:

 - #158410 (Update LLVM for Mach-O __LINKEDIT alignment fix.)
 - #157397 (cmse: clear padding when crossing the secure boundary)
 - #158036 (Add -Zinstrument-mcount=fentry to -Zinstrument-mcount)
 - #158330 (llvm: use intrinsics for f16, f32 minimum/maximum)
 - #158359 (fix(tests): allow either branch direction in ilog_known_base)
 - #158067 (LLVM 23: Adapt codegen test to moved assume)
 - #158261 (Move part of the target checking for `#[may_dangle]` to the parser)
 - #158358 (Fix invalid E0609 raw pointer deref suggestion inside macros)
 - #158392 (delegation: add tests for defaults and infers in generics)
 - #158394 (Generate synthetic generic args only for delegation's child segment)
@rust-bors rust-bors Bot merged commit e82d7b8 into rust-lang:main Jun 26, 2026
34 of 40 checks passed
@rustbot rustbot added this to the 1.98.0 milestone Jun 26, 2026
rust-timer added a commit that referenced this pull request Jun 26, 2026
@rust-timer
Unrolled build for #158330
6d31a69 
Rollup merge of #158330 - RalfJung:minimum-intrinsic, r=nikic

llvm: use intrinsics for f16, f32 minimum/maximum

The mentioned LLVM issues seem to only affect f64 and f128.

Cc @nikic @Urgau @tgross35
@RalfJung RalfJung deleted the minimum-intrinsic branch June 27, 2026 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tgross35 tgross35 tgross35 approved these changes

Assignees

@nikic nikic

Labels

A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. 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

Milestone

1.98.0

Development

Successfully merging this pull request may close these issues.

6 participants

@RalfJung @rustbot @rust-log-analyzer @nikic @tgross35 @nnethercote