RFC: StreamExecutor C API

[annarev]

The feedback phase will be open for two weeks until Tuesday June 30, 2020.

Status	Accepted
RFC #	257
Author(s)	Anna Revinskaya (annarev@google.com), Penporn Koanantakool (penporn@google.com), Yi Situ (yisitu@google.com), Russell Power (power@google.com)
Sponsor	Gunhan Gulsoy (gunan@google.com)
Updated	2020-09-08

Objective

Provide basic device management C API to allow new devices to modularly connect
to the current TensorFlow runtime.

[annarev]

I am planning to add the following SP_DeviceDescription struct. I made some fields optional and some required. @wchao1115, @kulinseth, @jzhoulon let me know if this looks reasonable (for e.g. if some other fields should be optional as well, such as pci_bus_id or if some optional fields should be required).

typedef struct SP_DeviceDescription {
  size_t struct_size;
  
  // Device hardware name. Used for printing.
  // Must be null-terminated.
  char* name;
  
  // Device vendor name. Used for printing.
  // Must be null-terminated.
  char* device_vendor;

  // Returns the PCI bus identifier for this device, of the form
  // [domain]:[bus]:[device].[function]
  // Used for printing.
  // Must be null-terminated.
  char* pci_bus_id;

  TF_Bool has_numa_node;  // True if `numa_node` is set.
  // Returns the NUMA node associated with this device, for use in
  // determining socket locality.
  int numa_node;

  TF_Bool has_memory_bandwidth;  // True if `memory_bandwidth` is set.
  // Device's memory bandwidth in bytes/sec.  (This is for reads/writes to/from
  // the device's own memory, not for transfers between the host and device.)
  int64_t memory_bandwidth;

  TF_Bool has_gflops;  // True if `gflops` field is set.
  // Estimate of floating point operations per second for this device * 10e-9.
  double gflops;
} SP_DeviceDescription;

[kulinseth]

I am planning to add the following SP_DeviceDescription struct. I made some fields optional and some required. @wchao1115, @kulinseth, @jzhoulon let me know if this looks reasonable (for e.g. if some other fields should be optional as well, such as pci_bus_id or if some optional fields should be required).

Thanks!. This looks good to me. Just to confirm, all the versioning information is subsumed in struct_size, and we don't need any extra fields in DeviceDescription for version compatibility. Is that correct?

[annarev]

I am planning to add the following SP_DeviceDescription struct. I made some fields optional and some required. @wchao1115, @kulinseth, @jzhoulon let me know if this looks reasonable (for e.g. if some other fields should be optional as well, such as pci_bus_id or if some optional fields should be required).

Thanks!. This looks good to me. Just to confirm, all the versioning information is subsumed in struct_size, and we don't need any extra fields in DeviceDescription for version compatibility. Is that correct?

Yes, we rely on struct_size and version value in SE_PlatformRegistrationParams.

[annarev]

I updated the doc to add a way to use a custom allocator:
21be6e9
Addition of a custom allocator was requested in Pluggable Device RFC:
#262

[penpornk]

This RFC has been up for a while. The implementation has even been checked in over a month ago. I think we should wrap this up (i.e., merge this PR).

Changes to StreamExecutor C API since the last combined design review meeting with PluggableDevice on 8/13/20:

• Added more explanation of the overall functionality.
• Added more clarifications on versioning strategy.
• Corrected typos. Minor functionality additions such as block_host_until_done, unified_memory_allocate/deallocate, etc.

If anyone has anything else that needs to be discussed/resolved before this RFC can be merged (i.e., fundamental issues), please explicitly say so here by this Thursday, 9/24 at 11:59pm PT.

[kulinseth]

If anyone has anything else that needs to be discussed/resolved before this RFC can be merged (i.e., fundamental issues), please explicitly say so here by this Thursday, 9/24 at 11:59pm PT.

Thanks Penporn, for reaching out. Nothing from our side.

[penpornk]

@kulinseth Thank you for the quick confirmation! :)

@ematejska @alextp I believe there is no outstanding issue now. Could we move forward?

[alextp]

Yes, let's merge the PR.

…

On Fri, Sep 25, 2020 at 10:13 AM Penporn Koanantakool < ***@***.***> wrote: @kulinseth <https://github.com/kulinseth> Thank you for the quick confirmation! :) @ematejska <https://github.com/ematejska> @alextp <https://github.com/alextp> I believe there is no outstanding issue now. Could we move forward? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#257 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAABHRL4JHCQHDI5DPSTMLLSHTFTRANCNFSM4N4SSC6Q> .

-- - Alex

[theadactyl]

LGTM

[wdfnst]

@penpornk Hi, is there any PluggableDevice template for beginner? TKS