Address more comments

Signed-off-by: Alexander Batashev <[email protected]>

Author: Alexander Batashev

sycl/source/detail/scheduler/commands.hpp

@@ -49,7 +49,7 @@ struct EnqueueResultT {
   EnqueueResultT(ResultT Result = SyclEnqueueSuccess, Command *Cmd = nullptr,
                  cl_int ErrCode = CL_SUCCESS)
       : MResult(Result), MCmd(Cmd), MErrCode(ErrCode) {}
-  /// Indicates result of enqueueing.
+  /// Indicates the result of enqueueing.
   ResultT MResult;
   /// Pointer to the command which failed to enqueue.
   Command *MCmd;
@@ -77,11 +77,11 @@ struct DepDesc {
   AllocaCommandBase *MAllocaCmd = nullptr;
 };
 
-/// The Command represents some action that needs to be performed on one or
-/// more memory objects. The Command has a vector of DepDesc objects that
-/// represent dependencies of the command. It has a vector of pointers to commands
-/// that depend on the command. It has a pointer to \ref queue object and an
-/// event that is associated with the command.
+/// The Command class represents some action that needs to be performed on one
+/// or more memory objects. The Command has a vector of DepDesc objects that
+/// represent dependencies of the command. It has a vector of pointers to
+/// commands that depend on the command. It has a pointer to a \ref queue object
+/// and an event that is associated with the command.
 ///
 /// \ingroup sycl_graph
 class Command {
@@ -112,8 +112,8 @@ class Command {
   /// Checks if the command is enqueued, and calls enqueueImp.
   ///
   /// \param EnqueueResult is set to the specific status if enqueue failed.
-  /// \param Blocking if this argument is true, function will wait for command
-  /// to be unblocked before calling enqueueImp.
+  /// \param Blocking if this argument is true, function will wait for the
+  ///        command to be unblocked before calling enqueueImp.
   /// \return true if the command is enqueued.
   bool enqueue(EnqueueResultT &EnqueueResult, BlockingT Blocking);
 
@@ -136,23 +136,23 @@ class Command {
   /// Looks at all the dependencies for the release command and enables
   /// instrumentation to report these dependencies as edges.
   void resolveReleaseDependencies(std::set<Command *> &list);
-  /// Creates an edge event when the dependency is a command
+  /// Creates an edge event when the dependency is a command.
   void emitEdgeEventForCommandDependence(Command *Cmd, void *ObjAddr,
                                          const string_class &Prefix,
                                          bool IsCommand);
-  /// Creates an edge event when the dependency is an event
+  /// Creates an edge event when the dependency is an event.
   void emitEdgeEventForEventDependence(Command *Cmd, RT::PiEvent &EventAddr);
-  /// Creates a signal event with the enqueued kernel event handle
+  /// Creates a signal event with the enqueued kernel event handle.
   void emitEnqueuedEventSignal(RT::PiEvent &PiEventAddr);
   /// Create a trace event of node_create type; this must be guarded by a
-  /// check for xptiTraceEnabled()
-  /// Post Condition: MTraceEvent will be set to the event created
-  /// \param MAddress  The address to use to create the payload
+  /// check for xptiTraceEnabled().
+  /// Post Condition: MTraceEvent will be set to the event created.
+  /// \param MAddress  The address to use to create the payload.
   uint64_t makeTraceEventProlog(void *MAddress);
   /// If prolog has been run, run epilog; this must be guarded by a check for
-  /// xptiTraceEnabled()
+  /// xptiTraceEnabled().
   void makeTraceEventEpilog();
-  /// Emits an event of Type
+  /// Emits an event of Type.
   void emitInstrumentation(uint16_t Type, const char *Txt = nullptr);
 
   // End Methods needed to support SYCL instrumentation
@@ -178,50 +178,51 @@ class Command {
   /// Private interface. Derived classes should implement this method.
   virtual cl_int enqueueImp() = 0;
 
-  /// The type of the command
+  /// The type of the command.
   CommandType MType;
   /// Mutex used to protect enqueueing from race conditions
   std::mutex MEnqueueMtx;
 
 public:
   /// Contains list of dependencies(edges)
   std::vector<DepDesc> MDeps;
-  /// Contains list of commands that depend on the command
+  /// Contains list of commands that depend on the command.
   std::unordered_set<Command *> MUsers;
-  /// Indicates whether the command can be blocked from enqueueing
+  /// Indicates whether the command can be blocked from enqueueing.
   bool MIsBlockable = false;
-  /// Counts the number of memory objects this command is a leaf for
+  /// Counts the number of memory objects this command is a leaf for.
   unsigned MLeafCounter = 0;
 
   const char *MBlockReason = "Unknown";
 
-  /// Describes the status of a command
+  /// Describes the status of the command.
   std::atomic<EnqueueResultT::ResultT> MEnqueueStatus;
 
   // All member variable defined here  are needed for the SYCL instrumentation
   // layer. Do not guard these variables below with XPTI_ENABLE_INSTRUMENTATION
   // to ensure we have the same object layout when the macro in the library and
   // SYCL app are not the same.
 
-  /// The event for node_create and task_begin
+  /// The event for node_create and task_begin.
   void *MTraceEvent = nullptr;
-  /// The stream under which the traces are emitted; stream ids are
-  /// positive integers and we set it to an invalid value
+  /// The stream under which the traces are emitted.
+  ///
+  /// Stream ids are positive integers and we set it to an invalid value.
   int32_t MStreamID = -1;
   /// Reserved for storing the object address such as SPIRV or memory object
-  /// address
+  /// address.
   void *MAddress = nullptr;
-  /// Buffer to build the address string
+  /// Buffer to build the address string.
   string_class MAddressString;
-  /// Buffer to build the command node type
+  /// Buffer to build the command node type.
   string_class MCommandNodeType;
-  /// Buffer to build the command end-user understandable name
+  /// Buffer to build the command end-user understandable name.
   string_class MCommandName;
-  /// Flag to indicate if makeTraceEventProlog() has been run
+  /// Flag to indicate if makeTraceEventProlog() has been run.
   bool MTraceEventPrologComplete = false;
-  /// Flag to indicate if this is the first time we are seeing this payload
+  /// Flag to indicate if this is the first time we are seeing this payload.
   bool MFirstInstance = false;
-  /// Instance ID tracked for the command
+  /// Instance ID tracked for the command.
   uint64_t MInstanceID = 0;
 };
 
@@ -242,8 +243,8 @@ class EmptyCommand : public Command {
   Requirement MRequirement;
 };
 
-/// The release command enqueues release of a memory object instance allocated on Host or
-/// underlying framework.
+/// The release command enqueues release of a memory object instance allocated
+/// on Host or underlying framework.
 class ReleaseCommand : public Command {
 public:
   ReleaseCommand(QueueImplPtr Queue, AllocaCommandBase *AllocaCmd);
@@ -286,7 +287,7 @@ class AllocaCommandBase : public Command {
   bool MIsActive = true;
 
   /// Indicates that the command owns memory allocation in case of connected
-  /// alloca command
+  /// alloca command.
   bool MIsLeaderAlloca = true;
 
 protected:
@@ -309,7 +310,7 @@ class AllocaCommand : public AllocaCommandBase {
   cl_int enqueueImp() final;
 
   /// The flag indicates that alloca should try to reuse pointer provided by
-  /// the user during memory object construction
+  /// the user during memory object construction.
   bool MInitFromUserData = false;
 };
 
@@ -329,7 +330,7 @@ class AllocaSubBufCommand : public AllocaCommandBase {
   AllocaCommandBase *MParentAlloca = nullptr;
 };
 
-/// The map command enqueues mapping of host memory onto device memory.
+/// The map command enqueues mapping of device memory onto host memory.
 class MapMemObject : public Command {
 public:
   MapMemObject(AllocaCommandBase *SrcAllocaCmd, Requirement Req, void **DstPtr,

sycl/source/detail/scheduler/scheduler.hpp

@@ -33,12 +33,13 @@
 ///
 /// The SYCL framework defines command group (\ref CG) as an entity that
 /// represents minimal execution block. The command group is submitted to SYCL
-/// queue and consists of a kernel and its requirements. The SYCL queue defines
-/// the device and context using which the kernel should be executed.
+/// queue and consists of a kernel or an explicit memory operation, and their
+/// requirements. The SYCL queue defines the device and context using which the
+/// kernel should be executed.
 ///
-/// There are also command groups that consist of memory requirements and
-/// an explicit memory operation, such as copy, fill, update_host. In this case
-/// it's up to an implementation how to implement these operations.
+/// The commands that contain explicit memory operations include copy, fill,
+/// update_host and other operations. It's up to implementation how to define
+/// these operations.
 ///
 /// The relative order of command groups submission defines the order in which
 /// kernels must be executed if their memory requirements intersect. For
@@ -93,17 +94,17 @@
 ///
 ///   // "Host accessor creation" section
 ///   // Request the latest data of BufferC for the moment
-///   // This is a synchronization point, which means that the DPC++ RT blocks on creation of
-///   // the accessor until requested data is available.
+///   // This is a synchronization point, which means that the DPC++ RT blocks
+///   // on creation of the accessor until requested data is available.
 ///   auto C = BufferC.get_access<read>();
 /// }
 /// \endcode
 ///
 /// In the example above the DPC++ RT does the following:
 ///
 /// 1. **Copy command group**.
-///    The DPC++ RT allocates memory for BufferA and BufferB on CPU then executes
-///   an explicit copy operation on CPU.
+///    The DPC++ RT allocates memory for BufferA and BufferB on CPU then
+///    executes an explicit copy operation on CPU.
 ///
 /// 2. **Multi command group**
 ///    DPC++ RT allocates memory for BufferC and BufferB on GPU and copy
@@ -266,8 +267,8 @@ struct MemObjRecord {
 /// executing the first command group memory allocation must be performed.
 ///
 /// At some point Scheduler enqueues commands to the underlying devices. To do
-/// this, Scheduler performs topological sort to get the order in which commands should
-/// be enqueued. For example, the following graph (D depends on B and C,
+/// this, Scheduler performs topological sort to get the order in which commands
+/// should be enqueued. For example, the following graph (D depends on B and C,
 /// B and C depends on A) will be enqueued in the following order:
 /// \code{.cpp}
 ///   EventA = Enqueue(A, /*Deps=*/{});
@@ -308,8 +309,7 @@ struct MemObjRecord {
 ///
 /// \section sched_impl Implementation details
 ///
-/// The Scheduler is split up into two parts: graph builder and graph
-/// processor.
+/// The Scheduler is split up into two parts: graph builder and graph processor.
 ///
 /// To build dependencies, Scheduler needs to memorize memory objects and
 /// commands that modify them.
@@ -338,9 +338,9 @@ struct MemObjRecord {
 /// 1. errors that happen during command enqueue process
 /// 2. the error that happend during command execution.
 ///
-/// If an error occurs during command enqueue process, the Command::enqueue method
-/// returns the faulty command. Scheduler then reschedules the command and all
-/// dependent commands (if any).
+/// If an error occurs during command enqueue process, the Command::enqueue
+/// method returns the faulty command. Scheduler then reschedules the command
+/// and all dependent commands (if any).
 ///
 /// An error with command processing can happen in underlying runtime, in this
 /// case Scheduler is notified asynchronously (using callback mechanism) what
@@ -378,26 +378,23 @@ class Scheduler {
 
   /// Removes buffer from the graph.
   ///
-  /// The lifetime of memory object descriptor begins when the first command group
-  /// that uses the memory object is submitted and ends when "removeMemoryObject(...)"
-  /// method is called which means there will be no command group that uses the
-  /// memory object. When removeMemoryObject is called Scheduler will enqueue
-  /// and wait on all release commands associated with the memory object, which
-  /// effectively guarantees that all commands accessing the memory object are
-  /// complete and then the resources allocated for the memory object are freed. Then all the
-  /// commands affecting the memory object are removed.
-  ///
-  /// On destruction Scheduler triggers destruction of all memory object
-  /// descriptors in order to wait on all commands not yet executed and all
-  /// memory it manages.
+  /// The lifetime of memory object descriptor begins when the first command
+  /// group that uses the memory object is submitted and ends when
+  /// "removeMemoryObject(...)" method is called which means there will be no
+  /// command group that uses the memory object. When removeMemoryObject is
+  /// called Scheduler will enqueue and wait on all release commands associated
+  /// with the memory object, which effectively guarantees that all commands
+  /// accessing the memory object are complete and then the resources allocated
+  /// for the memory object are freed. Then all the commands affecting the
+  /// memory object are removed.
   ///
   /// This member function is used by \ref buffer and \ref image.
   ///
   /// \param MemObj is a memory object that points to the buffer being removed.
   void removeMemoryObject(detail::SYCLMemObjI *MemObj);
 
-  /// Removes finished non-leaf non-alloca commands from the subgraph
-  /// (assuming that all its commands have been waited for).
+  /// Removes finished non-leaf non-alloca commands from the subgraph (assuming
+  /// that all its commands have been waited for).
   /// \sa GraphBuilder::cleanupFinishedCommands
   ///
   /// \param FinishedEvent is a cleanup candidate event.
@@ -458,13 +455,12 @@ class Scheduler {
     Command *addCGUpdateHost(std::unique_ptr<detail::CG> CommandGroup,
                              QueueImplPtr HostQueue);
 
-    /// Registers a \ref CG "command group" to update memory to the latest
-    /// state.
+    /// Enqueues a command to update memory to the latest state.
     ///
     /// \param Req is a requirement, that describes memory object.
     Command *addCopyBack(Requirement *Req);
 
-    /// Registers a \ref CG "command group" to create a host accessor.
+    /// Enqueues a command to create a host accessor.
     ///
     /// \param Req points to memory being accessed.
     Command *addHostAccessor(Requirement *Req, const bool destructor = false);
@@ -483,8 +479,9 @@ class Scheduler {
     /// Reschedules the command passed using Queue provided.
     ///
     /// This can lead to rescheduling of all dependent commands. This can be
-    /// used when the user provides a "secondary" queue to the submit method which may
-    /// be used when the command fails to enqueue/execute in the primary queue.
+    /// used when the user provides a "secondary" queue to the submit method
+    /// which may be used when the command fails to enqueue/execute in the
+    /// primary queue.
     void rescheduleCommand(Command *Cmd, QueueImplPtr Queue);
 
     /// \return a pointer to the corresponding memory object record for the
@@ -516,7 +513,8 @@ class Scheduler {
     std::vector<SYCLMemObjI *> MMemObjs;
 
   private:
-    /// Inserts the command required to update the memory object state in the context.
+    /// Inserts the command required to update the memory object state in the
+    /// context.
     ///
     /// Copy/map/unmap operations can be inserted depending on the source and
     /// destination.
@@ -579,26 +577,21 @@ class Scheduler {
   /// Member functions of this class do not modify the graph.
   ///
   /// \section sched_enqueue Command enqueueing
-  /// \todo lazy mode is not implemented.
-  ///
-  /// The Scheduler can work in two modes of enqueueing commands: eager (default)
-  /// and lazy. In eager mode commands are enqueued whenever they come to the
-  /// Scheduler. In lazy mode they are not enqueued until the content of the buffer
-  /// they are accessing is requested by user.
   ///
-  /// Each command has enqueue method which takes vector of events that
-  /// represents dependencies and returns event which represents the command.
-  /// GraphProcessor performs topological sort to get the order in which commands have to
-  /// be enqueued. Then it enqueues each command, passing a vector of events
-  /// that this command needs to wait on. If an error happens during command
-  /// enqueue, the whole process is stopped, the faulty command is propagated back
-  /// to the Scheduler.
+  /// Commands are enqueued whenever they come to the Scheduler. Each command
+  /// has enqueue method which takes vector of events that represents
+  /// dependencies and returns event which represents the command.
+  /// GraphProcessor performs topological sort to get the order in which
+  /// commands have to be enqueued. Then it enqueues each command, passing a
+  /// vector of events that this command needs to wait on. If an error happens
+  /// during command enqueue, the whole process is stopped, the faulty command
+  /// is propagated back to the Scheduler.
   ///
-  /// The command with dependencies that belong to a context different from its own
-  /// can't be enqueued directly (limitation of OpenCL runtime).
-  /// Instead, for each dependency, a proxy event is created in the target context
-  /// and linked using OpenCL callback mechanism with original one. For example,
-  /// the following SYCL code:
+  /// The command with dependencies that belong to a context different from its
+  /// own can't be enqueued directly (limitation of OpenCL runtime).
+  /// Instead, for each dependency, a proxy event is created in the target
+  /// context and linked using OpenCL callback mechanism with original one.
+  /// For example, the following SYCL code:
   ///
   /// \code{.cpp}
   ///   // The ContextA and ContextB are different OpenCL contexts