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

[SYCL] Remove re-flower pass #171

Merged
merged 4 commits into from
May 29, 2019

Conversation

Fznamznon
Copy link
Contributor

@Fznamznon Fznamznon commented May 27, 2019

Fixes #152

keryell
keryell previously approved these changes May 29, 2019
Copy link
Contributor

@keryell keryell left a comment

Choose a reason for hiding this comment

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

That looks good, if we consider this removes 15K SLOC...

#endif

#define PPCAT_NX(A, B) A ## B
#define PPCAT(A, B) PPCAT_NX(A, B)
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps adding an #undef at the end of the file for both of them too?

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've added #undef-s. Thanks!

Fznamznon added 4 commits May 29, 2019 09:23
Signed-off-by: Mariya Podchishchaeva <mariya.podchishchaeva@intel.com>
event_t and sampler_t type names are replaced with __ocl_event_t and
__ocl_sampler_t to avoid potential collisions with user types.

This change allows to re-use OpenCL event and sampler types for SYCL and replace
OpTypeEvent and OpTypeSampler with __ocl_event_t and __ocl_sampler_t in SYCL
headers.

OpenCL diagnostics is not enabled.

Signed-off-by: Mariya Podchishchaeva <mariya.podchishchaeva@intel.com>
This change allows to avoid re-flower pass using.

Signed-off-by: Mariya Podchishchaeva <mariya.podchishchaeva@intel.com>
Replace SPIR-V type names with built-in OpenCL type names.
Use mangled class names in IR checking.

Signed-off-by: Mariya Podchishchaeva <mariya.podchishchaeva@intel.com>
@Fznamznon Fznamznon force-pushed the private/mpodchis/remove-reflower branch 2 times, most recently from 4ad8e9b to 7287fff Compare May 29, 2019 09:39
@bader bader merged commit 39c82eb into intel:sycl May 29, 2019

// SPIRV type for sampler class
class OpTypeSampler;
// Only in such cases the class is recognized as SPIRV type __ocl_event_t.
Copy link
Contributor

Choose a reason for hiding this comment

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

Silly point to bring up, but should this read "Only in such cases the class is recognized as OpenCL type __ocl_event_t" rather than SPIRV?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we want SPIR-V translator to recognize __ocl_event_t as OpTypeEvent. Right?
Looks like another place where auto-replace should have been skipped.
@Fznamznon, please, rallback this change, if you and @agozillon do not object.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I understand. I have no objections.

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 think we should fully rewrite this comment (lines 39-42) because there is no more OCL C++ classes without definition. We use another approach now: we re-use OpenCL special opaque types in SYCL device compiler but we added aliases for these types to avoid potential conflicts with C++ code. We need the following typedefs only for host compiler because it doesn't know anything about these types.

@bader bader added the upstream This change is related to upstreaming SYCL support to llorg. label Jun 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upstream This change is related to upstreaming SYCL support to llorg.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SYCL] Remove re-flower pass
5 participants