Skip to content

[SYCL][E2E] Re-enable test case for exp(float) on win #16639

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

Merged
merged 5 commits into from
Mar 17, 2025

Conversation

KornevNikita
Copy link
Contributor

@KornevNikita KornevNikita commented Jan 15, 2025

The issue was fixed by #17440

@KornevNikita
Copy link
Contributor Author

KornevNikita commented Mar 6, 2025

Waiting for #16950
It's an MSVC bug.

@KornevNikita KornevNikita marked this pull request as ready for review March 6, 2025 13:00
@KornevNikita KornevNikita requested a review from a team as a code owner March 6, 2025 13:00
@KornevNikita KornevNikita linked an issue Mar 10, 2025 that may be closed by this pull request
@KornevNikita
Copy link
Contributor Author

KornevNikita commented Mar 11, 2025

The version of MSVC now is 14.43.34808.

This code:

#include <iostream>
#include <complex>
int main() {
  std::complex<float> t(0.5f, -0.f);
  std::complex<float> r = std::exp(t);
  std::cout << r << std::endl;
  return 0;
}

results in:

# .---command stdout------------
# | (1.64872,-0)
# `-----------------------------

The result is correct, so this compiler should be OK. But the test case is still failing:

# .---command stdout------------
# | Assertion std::signbit(r.imag()) == std::signbit(testcases[i].imag()) (line 337) failed for testcase #89
# | Assertion std::signbit(r.imag()) == std::signbit(testcases[i].imag()) (line 337) failed for testcase #90
# | Assertion std::signbit(r.imag()) == std::signbit(testcases[i].imag()) (line 337) failed for testcase #91
# | Assertion std::signbit(r.imag()) == std::signbit(testcases[i].imag()) (line 337) failed for testcase #92
# | Assertion std::signbit(r.imag()) == std::signbit(testcases[i].imag()) (line 337) failed for testcase #93
# | Assertion std::signbit(r.imag()) == std::signbit(testcases[i].imag()) (line 337) failed for testcase #94
# | Assertion std::signbit(r.imag()) == std::signbit(testcases[i].imag()) (line 337) failed for testcase #95
# | Assertion std::signbit(r.imag()) == std::signbit(testcases[i].imag()) (line 337) failed for testcase #96
# `-----------------------------

@jinge90 do you know what may be wrong?

@jinge90
Copy link
Contributor

jinge90 commented Mar 12, 2025

The version of MSVC now is 14.43.34808.

This code:

#include <iostream>
#include <complex>
int main() {
  std::complex<float> t(0.5f, -0.f);
  std::complex<float> r = std::exp(t);
  std::cout << r << std::endl;
  return 0;
}

results in:

# .---command stdout------------
# | (1.64872,-0)
# `-----------------------------

The result is correct, so this compiler should be OK. But the test case is still failing:

# .---command stdout------------
# | Assertion std::signbit(r.imag()) == std::signbit(testcases[i].imag()) (line 337) failed for testcase #89
# | Assertion std::signbit(r.imag()) == std::signbit(testcases[i].imag()) (line 337) failed for testcase #90
# | Assertion std::signbit(r.imag()) == std::signbit(testcases[i].imag()) (line 337) failed for testcase #91
# | Assertion std::signbit(r.imag()) == std::signbit(testcases[i].imag()) (line 337) failed for testcase #92
# | Assertion std::signbit(r.imag()) == std::signbit(testcases[i].imag()) (line 337) failed for testcase #93
# | Assertion std::signbit(r.imag()) == std::signbit(testcases[i].imag()) (line 337) failed for testcase #94
# | Assertion std::signbit(r.imag()) == std::signbit(testcases[i].imag()) (line 337) failed for testcase #95
# | Assertion std::signbit(r.imag()) == std::signbit(testcases[i].imag()) (line 337) failed for testcase #96
# `-----------------------------

@jinge90 do you know what may be wrong?

Let me take a look.
Thanks very much.

@jinge90
Copy link
Contributor

jinge90 commented Mar 12, 2025

Hi, @KornevNikita
I can reproduce the failure in my side and there are 2 findings:

  1. the failed cases are similar, the input complex's imag is '-0' and the result's imag is 0 which should be '-0', it looks like the signbit of return value is discarded.
  2. the failure is GPU specific, the test passed on CPU device in my side.

I will narrow down to see whether it is a IGC issue.

Thanks very much.

For input '-0', __spirv_ocl_sin returns +0 for float and returns
-0 for double, although no definition exists in standard for -0
input, MSVC complex math implementation does depend on this,
discarding the signbit for sin(-0) will lead to some corner case
failures in exp(std::complex<float>) e2e tests. This patch is a
workaround for the issue.
@KornevNikita
Copy link
Contributor Author

Reverted Ge's commit, it's already in the sycl - #17440

@KornevNikita
Copy link
Contributor Author

@intel/llvm-gatekeepers could you please merge?

@kbenzie kbenzie merged commit 2045895 into intel:sycl Mar 17, 2025
22 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.

Update MSVC on CI machines
4 participants