Skip to content

[SYCL] Remove re-flower pass #152

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
bader opened this issue May 21, 2019 · 4 comments · Fixed by #171
Closed

[SYCL] Remove re-flower pass #152

bader opened this issue May 21, 2019 · 4 comments · Fixed by #171
Assignees

Comments

@bader
Copy link
Contributor

bader commented May 21, 2019

Current approach with translation to SPIR-V uses re-flower pass. See this section for more details: https://github.com/intel/llvm/blob/sycl/sycl/doc/SYCL_compiler_and_runtime_design.md#integration-with-spir-v-format
To simplify SYCL compiler implementation and upstreaming work we should investigate if it's possible to translate SYCL to SPIR-V w/o this pass.

According to my understanding the main problem this pass solves is name conflicts.
For instance, type names defined for SPIR-V can conflict with user types - e.g. OpTypeEvent.

SPIR-V in LLVM IR format description: https://github.com/KhronosGroup/SPIRV-LLVM-Translator/blob/master/docs/SPIRVRepresentationInLLVM.rst

@Naghasan
Copy link
Contributor

Those types could be defined as a predefined types in Sema in a non conflicting way.

i.e. OpTypeEvent could become in the header file __spirv_OpTypeEvent. In clang's Sema, the init would predefine this type __spirv_OpTypeEvent when SYCL is enable to some internal type (maybe reuse the clang's OpenCL event type ?) which can then be lowered in a similar way as it is done for OpenCL in CodeGen.

One issue here: there is a conflicts between the mangling for SPIR and the SPIR-V convention.

@bader
Copy link
Contributor Author

bader commented May 27, 2019

@Naghasan, we are working on a bit different approach for SPIR-V types. Specifically for OpTypeEvent, we are trying to re-use OpenCL event_t in SYCL.
You can take the idea how it will be done from #167.
@Fznamznon, could you create a PR with your current state or update #167, please?

@Naghasan
Copy link
Contributor

That's very similar to what I had in mind, the problem will just be the mangler in the end (i.e. having a mangling that make sense for both OpenCL C and the SPIR-V translator).

@Fznamznon
Copy link
Contributor

@bader , @Naghasan , I've created PR with my changes - #171
I'm trying to re-use OpenCL event and sampler types -076d7a1

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 a pull request may close this issue.

3 participants