Skip to content

[SYCL][EXT] Define a new device selector that filters based on string input #2163

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

Closed
wants to merge 9 commits into from

Conversation

jbrodman
Copy link
Contributor

@jbrodman jbrodman commented Jul 22, 2020

string_selector takes a string that has the format "plat=Plat1,Plat2;type=Type1,Type2"
The platform strings should be something contained in platform.get_infoinfo::platform::name().
The device types atm are only either "cpu" or "gpu"

Valid inputs are:
""
"platform=Intel"
"type=cpu"
"platform=OpenCL;type=gpu,cpu"
and so forth.

Devices that match the supplied filters are then ranked according to the default selector.

Signed-off-by: James Brodman [email protected]

@jbrodman jbrodman requested a review from a team as a code owner July 22, 2020 21:40
jbrodman added 4 commits July 22, 2020 17:45
Signed-off-by: James Brodman <[email protected]>
Signed-off-by: James Brodman <[email protected]>
Signed-off-by: James Brodman <[email protected]>
int Score = REJECT_DEVICE_SCORE;

std::string CPU = "cpu";
std::string GPU = "gpu";
Copy link
Contributor

Choose a reason for hiding this comment

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

Are there plans to support accelerator(s) eventually as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

Or even FPGA :-), since it is a string and does not need anything predefined in SYCL specification...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this can be extended with more types in the future as necessary.

// RUN: %clangxx -fsycl -fsycl-targets=%sycl_triple %s -o %t1.out
// RUN: %t1.out

// REQUIRES: cpu, gpu, opencl
Copy link
Contributor

Choose a reason for hiding this comment

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

The checks below are written in quite safe manner, i.e. the test should pass with even only one of CPU or GPU is available.
Can this line be: "// REQUIRES: opencl" ? or be simply removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd prefer to check that it can do different things in the same program.

int Score = REJECT_DEVICE_SCORE;

std::string CPU = "cpu";
std::string GPU = "gpu";
Copy link
Contributor

Choose a reason for hiding this comment

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

Or even FPGA :-), since it is a string and does not need anything predefined in SYCL specification...

@bader
Copy link
Contributor

bader commented Aug 3, 2020

@rolandschulz, @Pennycook, any comments?

@romanovvlad, please, review and merge if it looks okay to you.

@Pennycook
Copy link
Contributor

I expect we're going to need to iterate on the syntax for this string a few times, especially given the existence of #2239. I think users are going to be confused if one string is formatted as platform=opencl,type=cpu and the other is formatted as cpu:opencl.

I don't feel strongly enough to block this PR, but tagging @jbrodman and @bso-intel to make them aware of both PRs.

@jbrodman jbrodman closed this Aug 3, 2020
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.

6 participants