Skip to content

Remove clamp debug_assert#263

Merged
Lokathor merged 1 commit into
Lokathor:mainfrom
Noam2Stein:remove-clamp-panic
May 9, 2026
Merged

Remove clamp debug_assert#263
Lokathor merged 1 commit into
Lokathor:mainfrom
Noam2Stein:remove-clamp-panic

Conversation

@Noam2Stein
Copy link
Copy Markdown
Contributor

When i added clamp i thought it should be consistent with the standard library and panic when min > max, max.is_nan or min.is_nan but i realized now that panics make it impossible to do branching via blend.

If this is some scalar code:

fn example(value: f32, min: f32, max: f32, should_clamp: bool) {
    if should_clamp {
        value.clamp(min, max)
    } else {
        value
    }
}

In that example it is ok to call example with NaNs when should_clamp is false, but for a SIMD version this will fail because clamp is still ran for all lanes:

fn example(value: f32x4, min: f32x4, max: f32x4, should_clamp: f32x4) {
    should_clamp.blend(
        value.clamp(min, max),
        value,
    )
}

The SIMD version will panic if any lane of min or max has NaNs even if should_clamp is false for that lane.

Too bad i did not think of this earlier.

@Lokathor
Copy link
Copy Markdown
Owner

Lokathor commented May 9, 2026

I see, so allowing a "nonsense" clamp is good because we might often immediately discard it in the next step anyway.

@Lokathor Lokathor merged commit a6346da into Lokathor:main May 9, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants