Address post-merge review comments on #1020 and use LLVM-style RTTI#1094
Address post-merge review comments on #1020 and use LLVM-style RTTI#1094EmilioLaiso wants to merge 8 commits intollvm:mainfrom
Conversation
| std::unique_ptr<offloadtest::Texture> RT; | ||
| std::unique_ptr<offloadtest::Buffer> RTReadback; | ||
| std::unique_ptr<offloadtest::Texture> DS; |
There was a problem hiding this comment.
Fantastic, using the abstract types brings InvocationState closer to being identical between render backends. The downcasting is needed for now, as we don't have a complete interface yet, but the places where we do cast act as TODO items for what has highest priority to be added to the interface.
| createFence(llvm::StringRef Name) = 0; | ||
|
|
||
| virtual llvm::Expected<std::shared_ptr<Buffer>> | ||
| virtual llvm::Expected<std::unique_ptr<Buffer>> |
There was a problem hiding this comment.
Just want to make a note that I still think we should go down the shared_ptr route. However that is currently being discussed in issue #1099.
I am okay with going unique_ptr for now because the keep alive system is not place yet. However, we are close to implementing and PRing that and will probably need to switch back to shared_ptr there.
EmilioLaiso
force-pushed
the
texture-and-buffers-as-unique-ptr
branch
from
5957622 to
61afed6
Compare
…re` construction protected
This reverts commit cd9a9fa.
EmilioLaiso
force-pushed
the
texture-and-buffers-as-unique-ptr
branch
from
61afed6 to
5f9a83c
Compare
bogner
left a comment
bogner
left a comment
There was a problem hiding this comment.
Generally looks good, just a style comment that applies throughout.
…, `Texture` and `CommandBuffer`
5 participants
Follow-up on the discussions at: #1020 (comment) & #1020 (comment)
This PR does the following:
offloadtest::Textureandoffloadtest::Buffertypes in backendsInvocationStateshared_ptrfor those types tounique_ptrBufferandTexture