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

[SPIRV] Add support of [[vk::ext_type_def]] #4068

Merged
merged 6 commits into from
Nov 29, 2021
Merged

Conversation

jiaolu
Copy link
Contributor

@jiaolu jiaolu commented Nov 8, 2021

this is related
#3919

@AppVeyorBot
Copy link

@AppVeyorBot
Copy link

@AppVeyorBot
Copy link

@AppVeyorBot
Copy link

@jaebaek jaebaek self-requested a review November 8, 2021 21:21
@jaebaek jaebaek added the spirv Work related to SPIR-V label Nov 8, 2021
@jaebaek
Copy link
Collaborator

jaebaek commented Nov 16, 2021

I am really sorry for being late. I am experiencing intensive workloads. I will review it until tomorrow. I know this is a complicated one and I appreciate your strong contribution!

Copy link
Collaborator

@jaebaek jaebaek left a comment

Choose a reason for hiding this comment

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

I will check how to define vk::ext_type, but could you please address my comments?
Let me know if you think sth of them are unclear (We can have email or video-call communications as well).

tools/clang/lib/SPIRV/EmitVisitor.cpp Show resolved Hide resolved
tools/clang/lib/SPIRV/SpirvEmitter.cpp Show resolved Hide resolved
@jaebaek
Copy link
Collaborator

jaebaek commented Nov 17, 2021

@jiaolu I sent a commit to your branch that allows us to put ext_type into vk namespace.

@AppVeyorBot
Copy link

@AppVeyorBot
Copy link

@AppVeyorBot
Copy link

@AppVeyorBot
Copy link

@AppVeyorBot
Copy link

@jiaolu
Copy link
Contributor Author

jiaolu commented Nov 29, 2021

hi, @jaebaek,
any comments to the latest change to the commits?

Copy link
Collaborator

@jaebaek jaebaek left a comment

Choose a reason for hiding this comment

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

Sorry for being late.
LGTM. I have just a last comment.

Thanks!

tools/clang/lib/SPIRV/SpirvContext.cpp Show resolved Hide resolved
@AppVeyorBot
Copy link

@jaebaek jaebaek merged commit 2eae8d3 into microsoft:master Nov 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spirv Work related to SPIR-V
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants