[SPIRV] Allow globallycoherent on DescriptorHeap accesses (#7740)#8513
[SPIRV] Allow globallycoherent on DescriptorHeap accesses (#7740)#8513SteveUrquhart wants to merge 1 commit into
Conversation
|
This addresses #7740 |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| // (a) An enclosing DeclStmt VarDecl initializer | ||
| // (b) A file-scope VarDecl initializer | ||
| // (c) A ReturnStmt inside a globallycoherent returning helper | ||
| auto isCoherentDest = [](const VarDecl *vd) { |
There was a problem hiding this comment.
Everywhere you use this vd is guaranteed to not be null, so this is just a lambda wrapping return true && fn(...).
We shouldn't use a lambda for this.
| // (a) and (c): walk Stmt parents through transparent wrappers. | ||
| { | ||
| const Stmt *cur = parentExpr; | ||
| while (cur) { |
There was a problem hiding this comment.
This approach is raising big red flags to me. This shouldn't be necessary. I think you're doing this to work around the AST not properly preserving the globally coherent attribute through implicit casts (which is a bug). I suspect this code gets a lot simpler and less buggy if we fix the issue in the AST rather than working around it.
There was a problem hiding this comment.
I posted #8583, which should fix the AST representation here so that the cast doesn't drop the attribute. This should make it so that the walking you're doing here is unnecessary.
2 participants
This PR allows the SpirvEmitter to emit globallycoherent in combination with ResourceDescriptorHeap.