Skip to content
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

[fix] OCL compiler flags for math tests on Integrated GPUs #553

Merged

Conversation

stratika
Copy link
Collaborator

@stratika stratika commented Sep 9, 2024

Description

This PR adds a compiler flag for OpenCL which enables math unit-tests to pass on Integrated GPUs.

This is for the Integrated GPUs that support FP64 operations. Otherwise, the message for unsupported operations is displayed and not an error in the result.

For example:

Running test: [34mtestMathAtan[0m               ................ [31m [FAILED] [0m
 		\_[REASON] expected:<0.7853981633974483> but was:<0.7470323434249905>

Note: I included only the -cl-opt-disable flag which was sufficient to compile the unit-tests that were breaking. There are more compiler flags that can be added via the DEFAULT_OPENCL_COMPILER_FLAGS. Some of them were available in previous commit points from the commit ffddb98 which initiated the refactoring. For example see the AbstractMetaData class.

Problem description

The refactoring of the compiler flags that was initiated in PR #539 had as an aftermath a change in the default OpenCL compiler flags. This change was not obvious for the majority of the devices, however it is breaking some math unit-tests for the Integrated Graphics (e.g., Intel UHD Graphics 630) with OpenCL.

Backend/s tested

Mark the backends affected by this PR.

  • OpenCL
  • PTX
  • SPIRV

OS tested

Mark the OS where this PR is tested.

  • Linux
  • OSx
  • Windows

Did you check on FPGAs?

If it is applicable, check your changes on FPGAs.

  • Yes
  • No

How to test the new patch?

Assuming that your Integrated graphics supports FP64 operations and it is device "0:1", you can run the following tests:

tornado --jvm "-Xmx6g -Dtornado.recover.bailout=False -Dtornado.unittests.verbose=True -Dtornado.ptx.priority=100 -Dtornado.unittests.device=0:1" -m tornado.unittests/uk.ac.manchester.tornado.unittests.tools.TornadoTestRunner --params "uk.ac.manchester.tornado.unittests.math.TestTornadoMathCollection"
tornado --jvm "-Xmx6g -Dtornado.recover.bailout=False -Dtornado.unittests.verbose=True -Dtornado.ptx.priority=100 -Dtornado.unittests.device=0:1" -m tornado.unittests/uk.ac.manchester.tornado.unittests.tools.TornadoTestRunner --params "uk.ac.manchester.tornado.unittests.math.TestMath"

@stratika stratika self-assigned this Sep 9, 2024
@stratika stratika added the bug Something isn't working label Sep 9, 2024
@stratika
Copy link
Collaborator Author

stratika commented Sep 9, 2024

After discussion with @jjfumero, we decided to remove the -cl-opt-disable from the default flags because overwrites and disables all other flags. Instead, we will use this disable compiler flag from the API method for the relevant unit-tests that assess accuracy (e.g. using the fast-math flag).

So, please hold on with the review till I do the update.

@stratika
Copy link
Collaborator Author

stratika commented Sep 9, 2024

I did the updates. I focused on the compiler flags for OpenCL. If the tests are failing for the other two backends, we will address them in a separate PR.

Please proceed with testing, and code review.

Copy link
Collaborator

@mairooni mairooni left a comment

Choose a reason for hiding this comment

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

LGTM

@jjfumero jjfumero merged commit cba529c into beehive-lab:develop Sep 10, 2024
2 checks passed
jjfumero added a commit to jjfumero/TornadoVM that referenced this pull request Sep 30, 2024
Improvements
============
- beehive-lab#565: New API call in the Execution Plan to log/trace the executed configuration plans.
- beehive-lab#563: Expand the TornadoVM profiler with Level Zero Sysman Energy Metrics.
- beehive-lab#559: Refactoring Power Metric handlers for PTX and OpenCL.
- beehive-lab#548: Benchmarking improvements.
- beehive-lab#549: Prebuilt API tests added using multiple backend-setup.
- Add internal tests for monitoring memory management [link](beehive-lab@0644225).

Compatibility
=============
- beehive-lab#561: Build for OSx 14.6 and OSx 15 fixed.

Bug Fixes
==============
- beehive-lab#564: Jenkins configuration fixed to run KFusion per backend.
- beehive-lab#562: Warmup action from the Execution Plan fixed to run with correct internal IDs.
- beehive-lab#557: Shared Execution Plans Context fixed.
- beehive-lab#553: OpenCL compiler flags for Intel Integrated GPUs fixed.
- beehive-lab#552: Fixed runtime to select any device among multiple SPIR-V devices.
- Fixed zero extend arithmetic operations: [link](beehive-lab@ea7b602).
@jjfumero jjfumero mentioned this pull request Sep 30, 2024
8 tasks
jjfumero added a commit to jjfumero/TornadoVM that referenced this pull request Sep 30, 2024
Improvements
============
- beehive-lab#565: New API call in the Execution Plan to log/trace the executed configuration plans.
- beehive-lab#563: Expand the TornadoVM profiler with Level Zero Sysman Energy Metrics.
- beehive-lab#559: Refactoring Power Metric handlers for PTX and OpenCL.
- beehive-lab#548: Benchmarking improvements.
- beehive-lab#549: Prebuilt API tests added using multiple backend-setup.
- Add internal tests for monitoring memory management [link](beehive-lab@0644225).

Compatibility
=============
- beehive-lab#561: Build for OSx 14.6 and OSx 15 fixed.

Bug Fixes
==============
- beehive-lab#564: Jenkins configuration fixed to run KFusion per backend.
- beehive-lab#562: Warmup action from the Execution Plan fixed to run with correct internal IDs.
- beehive-lab#557: Shared Execution Plans Context fixed.
- beehive-lab#553: OpenCL compiler flags for Intel Integrated GPUs fixed.
- beehive-lab#552: Fixed runtime to select any device among multiple SPIR-V devices.
- Fixed zero extend arithmetic operations: [link](beehive-lab@ea7b602).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Development

Successfully merging this pull request may close these issues.

3 participants