Skip to content

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