Draft of Metal driver api support#99
Conversation
| // Host memory associated with this buffer | ||
| T[] hostMemory; | ||
|
|
||
| this(T[] array) |
There was a problem hiding this comment.
this constructor should be a method of device
There was a problem hiding this comment.
I wanted to follow the approach done in CUDA driver.
currently the following is how clients call create buffer and implicitly it copies the buffer to device memory as well:
auto deviceX = Buffer!float(x);
auto deviceY = Buffer!float(y);
should it be as follows:
auto deviceX = Buffer!float(x);
auto deviceY = Buffer!float(y);
device.copy!float(deviceX);
device.copy!float(deviceY);
or should I remove Buffer completely and move the logic there to Device ?
There was a problem hiding this comment.
create buffer and implicitly it copies the buffer to device memory as well
then it should be a method of device: auto buf = device.makeBuffer!float(hostMemory);
| { | ||
| MTLComputePipelineState pipelineState; | ||
|
|
||
| this(MTLComputePipelineState ps) |
| { | ||
| MTLLibrary metalLibrary; | ||
|
|
||
| Device device; |
There was a problem hiding this comment.
why does the program contain a device?
The program represents the code.
It seems to me that you should split out the concept of a kernel from its binding to a pipeline state object.
| import metal.argument; | ||
| import metal.types; | ||
|
|
||
| struct Queue |
There was a problem hiding this comment.
a pipeline state seems much more associated with a Queue.
There was a problem hiding this comment.
yes agreed, I moved it to Queue as it fits better there
| // TODO(asadbek): explore options to make the use of async execution with events | ||
| this (Device _device /*bool async*/) | ||
| { | ||
| device = _device; |
There was a problem hiding this comment.
device is unused outside of the constructor, why cache a reference to it in the Queue?
There was a problem hiding this comment.
I don't know how to encode it in other way, I used device within enqueue method's OpCall which makes it functor type of function, I can add a new parameter to get the device but I thought it would be nice to encode it as field instead of another parameter in the enqueue function
… Queue as it fits better there and store MTLFunction in kernel metadata instead
No description provided.