i32.clamp() suggested by Clippy produces worse code than i32.min().max()
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 119k
- Forks
- 16.1k
- PR merge metrics
- PR metrics pending
Description
On this code in image-webp, following the Clippy lint to replace .max(0).min(255) with .clamp(0,255) on an i32 value causes a performance regression:
It's unfortunate that .min().max() and .clamp() are not equivalent, and doubly so when Clippy nags us to rewrite the code in a way that makes it slower.
I've posted a self-contained sample that reproduces the issue on godbolt:
Generated assembly for .min().max(): https://rust.godbolt.org/z/zr7PK8vz3
Generated assembly for .clamp(): https://rust.godbolt.org/z/b898M45vo
You can see that the .clamp() version results in far more assembly; the vectorized loop is roughly twice the amount of instructions.
I've confirmed that the issue exists in rustc 1.75, 1.82 and 1.87 which is the latest as of this writing.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the linked image-webp code at src/vp8.rs lines 994-999 and compare the two Godbolt samples for min().max() and clamp(). Investigate how rustc 1.75, 1.82, and 1.87 generate the differing assembly; done means identifying and correcting the compiler behavior without the clamp form causing the reported regression.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- compilers, performance
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100