Skip to content

[ESIMD] Break @llvm.fmuladd back into mul/add to satisfy VC BE. #5075

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 1 commit into from
Dec 4, 2021

Conversation

kbobrovs
Copy link
Contributor

@kbobrovs kbobrovs commented Dec 3, 2021

VC BE does not support many of std llvm intrinsics for now, so need a w/a.
This patch handles fmuladd.
This fixes regression on mandelbrot.cpp E2E test.
Signed-off-by: Konstantin S Bobrovsky [email protected]

@kbobrovs
Copy link
Contributor Author

kbobrovs commented Dec 3, 2021

With the latest pulldown default FE handling of FP changed and it started to generate fmuladd here.

@v-klochkov
Copy link
Contributor

Please check what is wrong with ESIMD/regression/dgetrf.cpp - it failed in CI, couldn't that be caused by this patch?

@kbobrovs
Copy link
Contributor Author

kbobrovs commented Dec 4, 2021

ESIMD failures in Jenkins are "unexpected pass"-es, which are expected - this patch fixes the regressions on them.
The rest of the failures are non-ESIMD, so they can't be related to this patch because it only touches llvm/lib/SYCLLowerIR/LowerESIMD.cpp.

[2021-12-03T06:26:18.803Z] Failed Tests (5):
[2021-12-03T06:26:18.803Z] SYCL :: DeviceLib/built-ins/vector_relational.cpp
[2021-12-03T06:26:18.803Z] SYCL :: DeviceLib/cmath_fp64_test.cpp
[2021-12-03T06:26:18.803Z] SYCL :: DeviceLib/cmath_test.cpp
[2021-12-03T06:26:18.803Z] SYCL :: DeviceLib/math_fp64_test.cpp
[2021-12-03T06:26:18.803Z] SYCL :: DeviceLib/math_test.cpp

[2021-12-03T06:09:03.533Z] Unexpectedly Passed Tests (3):
[2021-12-03T06:09:03.533Z] LLVM :: ESIMD/mandelbrot/mandelbrot.cpp
[2021-12-03T06:09:03.534Z] LLVM :: ESIMD/mandelbrot/mandelbrot_spec.cpp
[2021-12-03T06:09:03.534Z] LLVM :: ESIMD/regression/dgetrf.cpp

@kbobrovs
Copy link
Contributor Author

kbobrovs commented Dec 4, 2021

Please check what is wrong with ESIMD/regression/dgetrf.cpp - it failed in CI, couldn't that be caused by this patch?

It is also unexpected XPASS, the reason as the same as for mandelbrot - it started to generate fmuladd, was marked as XFAIL, now this patch fixes it.

@v-klochkov
Copy link
Contributor

Please check what is wrong with ESIMD/regression/dgetrf.cpp - it failed in CI, couldn't that be caused by this patch?

It is also unexpected XPASS, the reason as the same as for mandelbrot - it started to generate fmuladd, was marked as XFAIL, now this patch fixes it.

Ok, understood. BTW, then it makes sense fixing the LIT tests (remove XFAIL) and run the command/comment int this 5075 such line: "/verify with REFERENCE_TO_LIT_FIX_PR"

@kbobrovs
Copy link
Contributor Author

kbobrovs commented Dec 4, 2021

After re-running

Please check what is wrong with ESIMD/regression/dgetrf.cpp - it failed in CI, couldn't that be caused by this patch?

It is also unexpected XPASS, the reason as the same as for mandelbrot - it started to generate fmuladd, was marked as XFAIL, now this patch fixes it.

Ok, understood. BTW, then it makes sense fixing the LIT tests (remove XFAIL) and run the command/comment int this 5075 such line: "/verify with REFERENCE_TO_LIT_FIX_PR"

good point, will do

@kbobrovs
Copy link
Contributor Author

kbobrovs commented Dec 4, 2021

Rerunning the precommit left only the 3 "unexpected pass" failures above.

@kbobrovs
Copy link
Contributor Author

kbobrovs commented Dec 4, 2021

Accompanying test fix: intel/llvm-test-suite#600

@kbobrovs
Copy link
Contributor Author

kbobrovs commented Dec 4, 2021

"/verify with #5075" in intel/llvm-test-suite#600 did not work - it did not make CI pick this my patch for testing.

Nevertheless, "Unexepected pass" on the 3 tests w/o regression is 100% evidence that the patch does the right thing, so I'm merging.

@kbobrovs kbobrovs merged commit e800e7f into intel:sycl Dec 4, 2021
@kbobrovs kbobrovs deleted the esimd_fmuladd branch January 19, 2022 22:48
aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Mar 27, 2023
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.

2 participants