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

[SYCLomatic] USM_NONE testing fixes #243

Open
wants to merge 6 commits into
base: SYCLomatic
Choose a base branch
from

Conversation

danhoeflinger
Copy link
Contributor

@danhoeflinger danhoeflinger commented Feb 24, 2023

A number of onedpl_test_* tests assume the presence of USM.

This PR attempts to address the usage of dpct::device_vector and dpct::device_pointer and assumptions that USM is available in a few ways:

  1. In onedpl_test_copy_if, onedpl_test_fill.cpp, onedpl_test_sort_by_key.cpp, onedpl_test_for_each.cpp and onedpl_test_transform_reduce.cpp : we change the usage pattern of some tests to use dpct::device_vector in such a way to fit both USM and USM_NONE options

  2. In onedpl_test_exclusive_scan.cpp, onedpl_test_fill.cpp, and onedpl_test_sort_by_key.cpp: we add checks to avoid some tests when DPCT_USM_LEVEL_NONE is defined.

  3. This PR also adds 2 tests which were missing from help_function/help_function.xml: onedpl_test_device_malloc_free, and onedpl_test_transform, but only to run for the option where USM is present.
    Edit: removing onedpl_test_transform as it is failing in all cases it seems. It seems this should be dealt with separately.

  4. This PR also changes onedpl_test_uninitialized_fill to only run with USM available, as all of its tests assume the USM is available.

Closing #240 in favor of this PR which has a larger scope.

@danhoeflinger
Copy link
Contributor Author

It does not seem like using TEMPLATE_help_function_usm.xml actually prevents it from running with USM_NONE defined.

Am I misunderstanding how this should be used?

Perhaps this xml need to be updated with an appropriate rule to exclude USM_NONE?

I'm also surprised that onedpl_test_device_malloc_free and onedpl_test_uninitialized_fill are passing with USM_NONE defined, as they are using USM malloc together with dpct::device_pointer.

Copy link
Contributor

@tomflinda tomflinda left a comment

Choose a reason for hiding this comment

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

The changes are OK to me, pls make sure CI test pass and all conflicts resolved before merging.

@danhoeflinger danhoeflinger force-pushed the dev/dhoeflin/USM_NONE_testing_fixes branch from d726813 to dea1a1d Compare June 29, 2023 12:55
@danhoeflinger
Copy link
Contributor Author

The changes are OK to me, pls make sure CI test pass and all conflicts resolved before merging.

Thanks. I've rebased and resolved the conflicts. Some tests are failing, but it seems to be a configuration issue of the CUDA environment variables on Windows. Is this a known issue?

@danhoeflinger danhoeflinger force-pushed the dev/dhoeflin/USM_NONE_testing_fixes branch from 80ed71c to b47456b Compare August 16, 2023 20:53
@danhoeflinger danhoeflinger force-pushed the dev/dhoeflin/USM_NONE_testing_fixes branch from b47456b to 602503b Compare March 28, 2024 21:00
Copy link
Contributor

@tomflinda tomflinda left a comment

Choose a reason for hiding this comment

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

Pls rebase your PR to latest code repo to trigger the CI test again.

@danhoeflinger danhoeflinger force-pushed the dev/dhoeflin/USM_NONE_testing_fixes branch from 0ac5b11 to bebed30 Compare April 18, 2024 01:43
@danhoeflinger danhoeflinger force-pushed the dev/dhoeflin/USM_NONE_testing_fixes branch from bebed30 to 38cde29 Compare July 22, 2024 17:11
@danhoeflinger danhoeflinger force-pushed the dev/dhoeflin/USM_NONE_testing_fixes branch from 38cde29 to bc9ac29 Compare August 19, 2024 15:45
@danhoeflinger danhoeflinger force-pushed the dev/dhoeflin/USM_NONE_testing_fixes branch from bc9ac29 to 6e3ff93 Compare September 4, 2024 19:39
Signed-off-by: Dan Hoeflinger <[email protected]>
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.

3 participants