-
Notifications
You must be signed in to change notification settings - Fork 316
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
Add fp16 Intrinsics #510
Add fp16 Intrinsics #510
Conversation
9e4763d
to
7fb13f8
Compare
7fb13f8
to
154af26
Compare
CI failures seem spurious? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the patch, looks good. One minor thing to resolve before merge.
Convert from an i16 to the given FP type | ||
""" | ||
if not to: | ||
raise TypeError("expected a float return type") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this branch exercised in the tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added another test so all the error messages are covered now
This will also need a merge from master once the thing causing the test failures is fixed (probably #513) |
Close/Open to restart all of CI easily. Apologies for the noise. |
Thanks for the review @stuartarchibald, I've pushed fixes to address all your comments - |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fixes.
..because NVVM IR doesn't support half types!