Skip to content

[SYCL] Allow __spirv_ocl_printf vararg function #909

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 4 commits into from
Dec 9, 2019
Merged

[SYCL] Allow __spirv_ocl_printf vararg function #909

merged 4 commits into from
Dec 9, 2019

Conversation

asavonic
Copy link
Contributor

@asavonic asavonic commented Dec 5, 2019

Vararg functions are generally not allowed in SYCL. However, printf is
a special case, because it can be mapped to a SPIR-V instruction, and
SPIR-V translator recognizes __spirv_ocl_printf function.

Signed-off-by: Andrew Savonichev andrew.savonichev@intel.com

[SYCL] Allow __spirv_ocl_printf vararg function
fcd56c8 
Vararg functions are generally not allowed in SYCL. However, printf is
a special case, because it can be mapped to a SPIR-V instruction, and
SPIR-V translator recognizes __spirv_ocl_printf function.

Signed-off-by: Andrew Savonichev <andrew.savonichev@intel.com>
@asavonic asavonic mentioned this pull request Dec 5, 2019
Merged
erichkeane
erichkeane reviewed Dec 5, 2019
View reviewed changes
Andrew Savonichev added 2 commits December 6, 2019 14:40
Only allow a mangled global declaration
6faed7a 
Signed-off-by: Andrew Savonichev <andrew.savonichev@intel.com>
Make it const
f1c8c53 
Signed-off-by: Andrew Savonichev <andrew.savonichev@intel.com>
erichkeane
erichkeane reviewed Dec 6, 2019
View reviewed changes
Handle more cases of decl context
e7c10b7 
Signed-off-by: Andrew Savonichev <andrew.savonichev@intel.com>
@erichkeane
Copy link
Contributor

I don't seem to have the ability to review this patch anymore (no review button at the top), but LGTM.

@bader
Copy link
Contributor

bader commented Dec 9, 2019

I don't seem to have the ability to review this patch anymore (no review button at the top), but LGTM.

@tfzhu, any ideas what could be the reason for that?
@erichkeane, it sounds very strange, that you can't review the patch considering that you were able to review it before. What did you change? Another machine/browser?

@erichkeane
Copy link
Contributor

I don't seem to have the ability to review this patch anymore (no review button at the top), but LGTM.

@tfzhu, any ideas what could be the reason for that?
@erichkeane, it sounds very strange, that you can't review the patch considering that you were able to review it before. What did you change? Another machine/browser?

Nothing has changed besides the day :) The 'review' button just isn't on the interface today.

erichkeane
erichkeane approved these changes Dec 9, 2019
View reviewed changes
Copy link
Contributor

@erichkeane erichkeane left a comment

Choose a reason for hiding this comment

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

works now I guess...

Fznamznon
Fznamznon approved these changes Dec 9, 2019
View reviewed changes
Copy link
Contributor

@Fznamznon Fznamznon left a comment

Choose a reason for hiding this comment

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

I don't have objections.

@bader bader merged commit ddfe62a into intel:sycl Dec 9, 2019
iclsrc pushed a commit that referenced this pull request May 15, 2024
@psteinfeld
Revert "[flang] Initial debug info support for local variables. (#909……
4683571 
… (#92302)

…05)"

This reverts commit 61da636.

Update #90905 was causing many tests to fail.

See comments in #90905.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexeySachkov AlexeySachkov AlexeySachkov left review comments

@erichkeane erichkeane erichkeane approved these changes

@Fznamznon Fznamznon Fznamznon approved these changes

@bader bader Awaiting requested review from bader

Assignees

@Fznamznon Fznamznon

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@asavonic @erichkeane @bader @AlexeySachkov @Fznamznon