Skip to content

Conversation

@whitneywhtsang
Copy link
Contributor

@whitneywhtsang whitneywhtsang commented Nov 20, 2024

Before this PR, precise sqrt is lowered to __imf_sqrtf, which is not precise.
__imf_sqrt_rn is added in the new device library updated in #2774.
This PR uses __imf_sqrt_rn to get the precise sqrt implemented.

@whitneywhtsang whitneywhtsang self-assigned this Nov 20, 2024
@whitneywhtsang whitneywhtsang linked an issue Nov 20, 2024 that may be closed by this pull request
@whitneywhtsang whitneywhtsang changed the title [UT] Fix test_scan_layouts [UT] Fix test_precise_math Nov 20, 2024
Base automatically changed from whitneywhtsang/libdevice to main November 20, 2024 22:27
@whitneywhtsang whitneywhtsang requested a review from a team November 20, 2024 23:11
Copy link
Contributor

@victor-eds victor-eds left a comment

Choose a reason for hiding this comment

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

I may be missing some context (can you add PR description, please?), but I see our test is indeed failing and we are playing tricks to get it to pass. Would llvm/llvm-project#88222 (comment) fix the test without needing to change it?

@whitneywhtsang
Copy link
Contributor Author

I may be missing some context (can you add PR description, please?), but I see our test is indeed failing and we are playing tricks to get it to pass.

Added PR description.

Would llvm/llvm-project#88222 (comment) fix the test without needing to change it?

That is a different issue, we currently need to use cpu result as reference, or else we are comparing with the imprecise result.

Copy link
Contributor

@victor-eds victor-eds left a comment

Choose a reason for hiding this comment

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

LGTM keeping as is after offline conversation

Signed-off-by: Whitney Tsang <[email protected]>
Signed-off-by: Whitney Tsang <[email protected]>
@whitneywhtsang whitneywhtsang merged commit 2aae978 into main Nov 22, 2024
5 checks passed
@whitneywhtsang whitneywhtsang deleted the whitneywhtsang/sqrt_rn branch November 22, 2024 15:01
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.

tl.math.sqrt_rn(x) runs incorrectly on XPU

5 participants