Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix min_float32 for parsing float literals #4229

Closed
wants to merge 1 commit into from

Conversation

cchan
Copy link

@cchan cchan commented Jun 30, 2024

The minimum normal 2**-126, but the minimum denormal is 2**-23 times smaller than that.

@cchan cchan requested a review from ptillet as a code owner June 30, 2024 06:44
@jlebar jlebar enabled auto-merge (squash) June 30, 2024 07:18
Copy link
Collaborator

@jlebar jlebar left a comment

Choose a reason for hiding this comment

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

On second thought, I'm not sure about this PR.

Subnormal values lose precision. I think if we want to accept them as float32's, we should also check that they are exactly representable as float32?

@jlebar
Copy link
Collaborator

jlebar commented Jun 30, 2024

exactly representable as float32?

I grant this is also a little weird because if your input is e.g. 1/3, that will be represented as a python f64, and it's not exactly-representable as a f32. But we'll still cast it to a Triton f32. So why have a special case for subnormals? Except I feel like it's one thing to lose half the mantissa bits of 1/3, while it's another thing to lose potentially all but one of the mantissa bits of a f64 that is represented as an f32 subnormal.

@ThomasRaoux
Copy link
Collaborator

I agree with Justin, in addition to that one thing to consider is that this breaks backward compatibility and may break existing kernels in a non trivial way.

@cchan
Copy link
Author

cchan commented Jul 1, 2024

Hmmm, those are fair points. There should be a way to produce custom constexpr types somehow, though.

e.g. what about something like this?

x : tl.constexpr[tl.float32] = FLOAT32_MIN_POSITIVE
y : tl.constexpr[tl.float64] = 1/3

@ThomasRaoux
Copy link
Collaborator

Hmmm, those are fair points. There should be a way to produce custom constexpr types somehow, though.

e.g. what about something like this?

x : tl.constexpr[tl.float32] = FLOAT32_MIN_POSITIVE
y : tl.constexpr[tl.float64] = 1/3

can you just assign the constexpr and do tl.cast to get to the type you want?

@cchan
Copy link
Author

cchan commented Jul 4, 2024

I did what y'all suggested, and it seems to have been smart enough to do the right thing, turning it into an immediate.

        mul.f32         %f9, %f8, 0f07800000;
        mul.f32         %f10, %f7, 0f07800000;
        mul.f32         %f11, %f6, 0f07800000;
        mul.f32         %f12, %f5, 0f07800000;
        mul.f32         %f13, %f4, 0f07800000;
        mul.f32         %f14, %f3, 0f07800000;
        mul.f32         %f15, %f2, 0f07800000;
        mul.f32         %f16, %f1, 0f07800000;

Thanks both :)

@cchan cchan closed this Jul 4, 2024
auto-merge was automatically disabled July 4, 2024 09:29

Pull request was closed

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.

3 participants