rust-lang / rust-lang/rust

i32.clamp() suggested by Clippy produces worse code than i32.min().max()

Open
#141,915 7 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

A-LLVM C-optimization I-slow T-compiler
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:

https://github.com/image-rs/image-webp/blob/93baf7de7df50977a1fcb3a0bb53036d4780bff3/src/vp8.rs#L994-L999

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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.