Skip to content

Conversation

Fznamznon
Copy link
Contributor

This attribute contains string with module identifier. This information
will be used to split fully linked device modules into smaller ones.

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

@Fznamznon Fznamznon requested a review from kbobrovs October 3, 2019 10:01
@Fznamznon
Copy link
Contributor Author

This is part of device code splitting feature. Please refer #631 for full design doc.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it reliable way to get unique id for the module? Can't we have two different modules with the same module id?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a valid concern. It seems hard to come up with 100% reliable ID, so at least a TODO should be added to the code.
Other ideas to make it more reliable: add compilation time, addresses of Function objects corresponding to kernels in this translation unit.

Not 100% reliability should be OK, though, as the worst result is kernels going to the same module rather than different ones.

Copy link
Contributor

Choose a reason for hiding this comment

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

Added a TODO comment to address this concern later.

Fznamznon and others added 2 commits October 8, 2019 12:12
This attribute contains string with module identifier. This information
will be used to split fully linked device modules into smaller ones.

Signed-off-by: Mariya Podchishchaeva <[email protected]>
Signed-off-by: Sergey Semenov <[email protected]>
Signed-off-by: Sergey Semenov <[email protected]>
@sergey-semenov sergey-semenov force-pushed the private/mpodchis/module-id-attr branch from c5f18db to 885515a Compare October 8, 2019 12:09
@sergey-semenov
Copy link
Contributor

Per our agreement with Mariya, I'll be driving this PR for now.

@sergey-semenov sergey-semenov force-pushed the private/mpodchis/module-id-attr branch from 885515a to 6e06ec8 Compare October 8, 2019 12:14
@Fznamznon
Copy link
Contributor Author

Ping.

@Fznamznon Fznamznon changed the title [SYCL] Add generation of module-id attribute for kernels [SYCL] Add generation of sycl-module-id attribute for kernels Oct 14, 2019
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

@romanovvlad romanovvlad merged commit 6954406 into intel:sycl Oct 15, 2019
vladimirlaz pushed a commit to vladimirlaz/llvm that referenced this pull request Aug 31, 2020
New option allows to specify which version of extended
instruction set for representing debug info should be used
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.

5 participants