Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[HLSL] Implement explicit layout for default constant buffer ($Globals) #128991
base: users/hekota/pr-128981-res-binding-on-numeric-types
Are you sure you want to change the base?
[HLSL] Implement explicit layout for default constant buffer ($Globals) #128991
Changes from 2 commits
e982a61
458d5f0
22d9d1f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'd be nice if there was a named constant for this. At very least you wouldn't need the comment. At best, we have an easy way to find all the places that depend on the constant buffer row size.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there's a
CBufferAlign
in SemaHLSL.cpp. ABufferRowAlign
in HLSLBufferLayoutBuilder.cpp. Maybe these are the same concept?(If so, maybe reconciling them is outside scope of this PR)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've named the constant and placed it in CGHLSLRuntime. Codegen does not have a dependency on Sema.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I think this is probably fine here, but I wonder more generally if there's any way we can get Sema and CG to agree on constants and things like that? Does Sema depend on Codegen?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, Sema does (and should not) depend on CodeGen. The only dependency between Sema and CodeGen are the AST nodes. Maybe we could have a shared header with constants in llvm/lib/Frontend/HLSL. I'll think about it.