Skip to content

[SYCL][Driver] Add device code split job #853

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 13 commits into from
Nov 28, 2019
Merged

[SYCL][Driver] Add device code split job #853

merged 13 commits into from
Nov 28, 2019

Conversation

Fznamznon
Copy link
Contributor

No description provided.

@Fznamznon Fznamznon requested review from mdtoguchi, AGindinson and kbobrovs and removed request for mdtoguchi and AGindinson November 20, 2019 15:29
@Fznamznon
Copy link
Contributor Author

Please review only second commit. Review for first is here - #793 .

AGindinson
AGindinson reviewed Nov 20, 2019
View reviewed changes
AGindinson
AGindinson reviewed Nov 21, 2019
View reviewed changes
AGindinson
AGindinson reviewed Nov 21, 2019
View reviewed changes
@AGindinson AGindinson self-assigned this Nov 22, 2019
mdtoguchi
mdtoguchi reviewed Nov 22, 2019
View reviewed changes
@Fznamznon Fznamznon force-pushed the private/mpodchis/driver-split-final branch from c2c8739 to 270d457 Compare November 25, 2019 12:17
AGindinson
AGindinson reviewed Nov 25, 2019
View reviewed changes
Copy link
Contributor

@AGindinson AGindinson left a comment

Choose a reason for hiding this comment

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

The rework looks good! Some minor remaining stuff.

AGindinson
AGindinson reviewed Nov 25, 2019
View reviewed changes
AGindinson
AGindinson previously approved these changes Nov 25, 2019
View reviewed changes
Copy link
Contributor

@AGindinson AGindinson left a comment

Choose a reason for hiding this comment

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

LGTM

@Fznamznon
Copy link
Contributor Author

Thanks @AGindinson for review.
@kbobrovs , @mdtoguchi , I'd like to get a feedback from you too.

@bader bader requested a review from mdtoguchi November 25, 2019 20:37
kbobrovs
kbobrovs reviewed Nov 26, 2019
View reviewed changes
Copy link
Contributor

@kbobrovs kbobrovs left a comment

Choose a reason for hiding this comment

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

LGTM, except that the default should be no split, IMO - see review comments

@Fznamznon
[SYCL][Driver] Add device code split job
595b9cd 
Signed-off-by: Mariya Podchishchaeva <mariya.podchishchaeva@intel.com>
@Fznamznon
Apply minor comments
4960e10 
Signed-off-by: Mariya Podchishchaeva <mariya.podchishchaeva@intel.com>
@Fznamznon
Apply comments
6a5eff7 
Signed-off-by: Mariya Podchishchaeva <mariya.podchishchaeva@intel.com>
@Fznamznon
Apply minor CR comments
bbcd3b5 
Signed-off-by: Mariya Podchishchaeva <mariya.podchishchaeva@intel.com>
@Fznamznon
Re-use tempfilelist instead of tempirfileslist
69b70c5 
Signed-off-by: Mariya Podchishchaeva <mariya.podchishchaeva@intel.com>
@Fznamznon
Do not perform split by default
aa34e25 
Signed-off-by: Mariya Podchishchaeva <mariya.podchishchaeva@intel.com>
@Fznamznon
Extend a comment
169ee4b 
Signed-off-by: Mariya Podchishchaeva <mariya.podchishchaeva@intel.com>
@Fznamznon Fznamznon force-pushed the private/mpodchis/driver-split-final branch from ab0591e to 169ee4b Compare November 27, 2019 17:05
@Fznamznon
Merge split options
33d8a4d 
Signed-off-by: Mariya Podchishchaeva <mariya.podchishchaeva@intel.com>
@Fznamznon
Make help for new options more detailed
6b9f252 
Signed-off-by: Mariya Podchishchaeva <mariya.podchishchaeva@intel.com>
kbobrovs
kbobrovs reviewed Nov 27, 2019
View reviewed changes
kbobrovs
kbobrovs reviewed Nov 27, 2019
View reviewed changes
kbobrovs
kbobrovs previously approved these changes Nov 27, 2019
View reviewed changes
Copy link
Contributor

@kbobrovs kbobrovs left a comment

Choose a reason for hiding this comment

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

LGTM

@Fznamznon
Apply suggestion
5e08fb9 
Signed-off-by: Mariya Podchishchaeva <mariya.podchishchaeva@intel.com>
AGindinson
AGindinson reviewed Nov 28, 2019
View reviewed changes
AGindinson
AGindinson previously approved these changes Nov 28, 2019
View reviewed changes
Copy link
Contributor

@AGindinson AGindinson left a comment

Choose a reason for hiding this comment

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

LGTM as well, thanks!

@Fznamznon
Fix comments
a088384 
Signed-off-by: Mariya Podchishchaeva <mariya.podchishchaeva@intel.com>
@Fznamznon
Fix comment
f2d616f 
Signed-off-by: Mariya Podchishchaeva <mariya.podchishchaeva@intel.com>
@Fznamznon
Fix comments again
a2782c1 
Signed-off-by: Mariya Podchishchaeva <mariya.podchishchaeva@intel.com>
@Fznamznon
Copy link
Contributor Author

It's time to merge, I guess.

bader
bader reviewed Nov 28, 2019
View reviewed changes
AGindinson
AGindinson reviewed Nov 28, 2019
View reviewed changes
@bader bader merged commit a339d4c into intel:sycl Nov 28, 2019
@Fznamznon Fznamznon mentioned this pull request Nov 28, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AGindinson AGindinson AGindinson left review comments

@bader bader bader approved these changes

@kbobrovs kbobrovs kbobrovs left review comments

@mdtoguchi mdtoguchi Awaiting requested review from mdtoguchi

Assignees

@kbobrovs kbobrovs

@AGindinson AGindinson

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Fznamznon @bader @kbobrovs @AGindinson @mdtoguchi