Skip to content

Conversation

@Fznamznon
Copy link
Contributor

Signed-off-by: Mariya Podchishchaeva [email protected]

@Fznamznon
Copy link
Contributor Author

Sorry, I forgot the test for #853 . My bad :(

@Fznamznon
Copy link
Contributor Author

Fznamznon commented Nov 28, 2019

Does anybody know why all the sycl driver tests are xfailed on windows? Even windows-specific test.

// XFAIL: windows-msvc

My test doesn’t seem failing with split-specific problem.

@AGindinson
Copy link
Contributor

Does anybody know why all the sycl driver tests are xfailed on windows? Even windows-specific test.

// XFAIL: windows-msvc

My test doesn’t seem failing with split-specific problem.

I can't analyze this properly now, but it seems related to some Windows-unfriendly path regexp.

Definitely not caused by your patch, but I'd like some input from @mdtoguchi or @v-klochkov before we blissfully add the same TODO + XFAIL and commit the test.

@Fznamznon
Copy link
Contributor Author

Definitely not caused by your patch, but I'd like some input from @mdtoguchi or @v-klochkov before we blissfully add the same TODO + XFAIL and commit the test.

I wanted to check that all tools is called properly when device code split is requested. So I took existing test cases and extended these test cases for device code split case. The problem happens in lines not related to split, so another option is reduce test cases to make them check only split-affected jobs.

@AGindinson
Copy link
Contributor

AGindinson commented Nov 29, 2019

The problem happens in lines not related to split, so another option is reduce test cases to make them check only split-affected jobs.

Given the feature complexity, I'd personally steer away from reducing coverage. The least risky option seems to be XFAIL: windows-msvc, and a later follow-up with either making the test "split-only" or analyzing + fixing the Windows problem, depending on others' opinions.

@Fznamznon
Copy link
Contributor Author

The problem happens in lines not related to split, so another option is reduce test cases to make them check only split-affected jobs.

Given the feature complexity, I'd personally steer away from reducing coverage. The least risky option seems to be XFAIL: windows-msvc, and a later follow-up with either making the test "split-only" or analyzing + fixing the Windows problem, depending on others' opinions.

How about add XFAIL and create a GitHub issue?

@AGindinson
Copy link
Contributor

How about add XFAIL and create a GitHub issue?

That's a good idea, I'll create one.

@Fznamznon Fznamznon force-pushed the private/mpodchis/driver-split-test branch from 28f09d4 to f38fff6 Compare November 29, 2019 14:21
@Fznamznon Fznamznon changed the title [SYCL][Driver] Add forgotten test for device code split job [SYCL][Driver] Add none value for device code split option Nov 29, 2019
@Fznamznon
Copy link
Contributor Author

How about add XFAIL and create a GitHub issue?

That's a good idea, I'll create one.

Okay then, I XFAILed the test.

This will help disable device code split if some previous option enabled
it.

Signed-off-by: Mariya Podchishchaeva <[email protected]>
@Fznamznon Fznamznon force-pushed the private/mpodchis/driver-split-test branch from f38fff6 to f9b664a Compare November 30, 2019 10:23
@Fznamznon Fznamznon changed the title [SYCL][Driver] Add none value for device code split option [SYCL][Driver] Add off value for device code split option Nov 30, 2019
Copy link
Contributor

@bader bader left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@bader bader merged commit 7931ce6 into intel:sycl Dec 2, 2019
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