[SYCL][PI] Removal of PI_CALL family of macros & implementation of Plugin_impl class #1030

garimagu · 2020-01-21T01:03:03Z

The changeset removes the family of PI_CALL macros, and replaces them with
call API in a new plugin_impl class. The object of this class is stored in
Platform class. When multiple platforms are associated with the same Plugin,
they will have shared_ptr to the same plugin_impl object. Eg: An OpenCLPlugin
can have multiple platforms attached to it.

To effectively use this change, following additional fetaures/changes
were added:

To make access to the plugin_impl class easy, getPlugin() API has been
introduced to some *_impl classes, that eventually call the
platform_impl->getPlugin() API. It has been included in context_impl,
event_impl, kernel_impl, platform_impl, program_impl, queue_impl. Is not
present in accessor_impl(Does not call PI), buffer_impl (Needs only in the
constructor where context is passed), sampler_impl (Does not have a single
context), stream_impl (Does not call PI), usm_impl (Has no member sycl_impl
objects.) The API returns const reference to plugin_impl class.
Use of PiApiKind enum to uniquely identify the PI API.
PI API calls are done using : plugin_impl.call, plugin_impl.call_nocheck,
plugin_impl.checkPiResult
GlobalPlugin is a SharedPtr to a globally available Plugin for the use with
Interoperability Constructors. It is used as a shared_ptr to avoid making
copies of it.
Changes in *_info structs and get() methods. They now take a const
plugin_impl& argument.
To make Pi Api calls by host to check if certain events are finished on a
device, the plugin_impl class is needed. At such places, event_impl is passed
instead of a PiEvent. Eg: memory_manager.cpp
The use of Global variable has been restricted to only the GlobalPlugin now.

Signed-off-by: Garima Gupta garima.gupta@intel.com

NOTE TO REVIEWERS:

Only platform_impl has ownership of the Plugin_impl object. Others only call the getPlugin() method on the platform_impl object.
RT::initialize() is called only once in getPlatforms() now. It recognizes all platforms associated with all attached plugins.
GlobalPlugin is a SharedPtr because if a global variable is kept, we will have to pass it by value or a pointer, thus making a shared_ptr takes care of managing the copies. This is the only global we have now for PI.
*_info structs and get methods: They now take a const Plugin_impl& argument. The methods were not changed to take _impl objects because the structs::get methods seem to be utility functions to get information given a Pi Object. Eg: get_kernel_infoinfo::kernel::program::get(PiKernel) method. To make these methods take *impl objects, some places may require creating a new *_impl object which would be incorrect.
memory_manager.cpp changes: Need to get the plugin information from individual events, since a host Context may wait on events of other devices; To enable this, event_impls are passed where a PI_CALL is needed.
Note that the plugin_information is only passed as const reference to any function if a sycl *_impl is not already available.
Test case that exists: llvm/sycl/unittests/pi/PlatformTest.cpp.
Please pay attention to any wrong extraction of plugin_information. Eg: A PI_CALL on event can be called on a host_context. I may have missed similar things at other places. Please point them out if you see any issue.

This patch does not take care of the following.

The code currently cannot connect to multiple plugins/or recognize and choose one among many. It will be enabled in the next few patches.
Next set of patches :
*To add the config file mechanism to find attached Plugins, as has been mentioned in [SYCL][DOC] Update the SYCL Runtime Interface document to give some design updates #680
*To add better tracing and debugging.
*Format the code in Doxygen friendly-way and move the implementation into cpp files if possible.

bader · 2020-01-21T10:18:42Z

@smaslov-intel, @kbobrovs, it looks like implementation of plug-in interface matured enough to fill some of design unknowns. Could you update plug-in documentation and replace TBDs with the implemented design, please?

sycl/source/detail/memory_manager.cpp

romanovvlad · 2020-01-21T11:23:02Z

sycl/include/CL/sycl/detail/kernel_program_cache.hpp

@@ -78,6 +82,7 @@ class KernelProgramCache {

  ProgramCacheT MCachedPrograms;
  KernelCacheT MKernelsPerProgramCache;
+  PlatformImplPtr MPlatform;


It seems very strange to have platform object in the cache.
@s-kanaev Do you know if we can do better?

As far as I understand cache only requires plugin information, not the whole platform. I'd prefer to keep plugin reference/pointer instead of platform.

Besides., is it even possible to retrieve plugin information out of PI kernel/program object/pointer?

It seems very strange to have platform object in the cache.
@s-kanaev Do you know if we can do better?

plugin_impl is only kept in Platform object.
To access it, platform_impl has been included here. The platform_impl here is from the context_impl to which the cache belongs.
Will it be better to include a reference to the parent Context? I wasn't able to achieve it in the code. So i decided to pass the platform the context belongs to.

As far as I understand cache only requires plugin information, not the whole platform. I'd prefer to keep plugin reference/pointer instead of platform.

Yes, only the plugin information is needed. The overall design takes care that only platform_impl owns a shared_ptr to the plugin, and others get a reference to the plugin only through platform_impl class. Storing a reference to plugin_impl has not been done anywhere. So I refrained from storing it here.

Besides., is it even possible to retrieve plugin information out of PI kernel/program object/pointer?

No we cannot access the plugin_information through any PI class objects. Atleast not with the current design.

Will it be better to include a reference to the parent Context? I wasn't able to achieve it in the code. So i decided to pass the platform the context belongs to.

This may be quite reasonable.
One may store a reference to parent context as well as a mere pointer. Besides, pointer will be wrapped with weak_ptr.

hmmm.. Let me try.. I'll update the code.
Thank you for pointing out the weak_ptr here. I didn't know about the circular dependency issue.

I have included the ptr instead of weak_ptr.

sycl/include/CL/sycl/detail/plugin_impl.hpp

garimagu · 2020-01-22T01:00:24Z

@smaslov-intel Please review.

sycl/include/CL/sycl/detail/kernel_program_cache.hpp

s-kanaev · 2020-01-22T09:42:50Z

sycl/include/CL/sycl/detail/kernel_program_cache.hpp

@@ -78,6 +82,7 @@ class KernelProgramCache {

  ProgramCacheT MCachedPrograms;
  KernelCacheT MKernelsPerProgramCache;
+  PlatformImplPtr MPlatform;


As far as I understand cache only requires plugin information, not the whole platform. I'd prefer to keep plugin reference/pointer instead of platform.

Besides., is it even possible to retrieve plugin information out of PI kernel/program object/pointer?

sycl/include/CL/sycl/detail/pi.hpp

s-kanaev · 2020-01-22T10:36:24Z

sycl/include/CL/sycl/detail/kernel_program_cache.hpp

@@ -78,6 +82,7 @@ class KernelProgramCache {

  ProgramCacheT MCachedPrograms;
  KernelCacheT MKernelsPerProgramCache;
+  PlatformImplPtr MPlatform;


Is it possible to have multiple plugins/platforms during the same runtime? Can we make plugin a singleton?

Currently no. But in the future yes. The design enables you to connect to multiple Plugins and have multiple platforms from different plugins at the same time.
We have a GlobalPlugin which stores one plugin in use,it is currently used for OpenCL Interoperability constructors only. It will be a bad design to use it here, because we would like to delete that GlobalPlugin in the future.

garimagu · 2020-01-23T19:44:35Z

@bjoernknafla @keryell
Thought i will add you in case you would like to review.

sycl/include/CL/sycl/detail/plugin_impl.hpp

sycl/source/detail/memory_manager.cpp

sycl/include/CL/sycl/detail/kernel_program_cache.hpp

garimagu · 2020-01-23T20:17:08Z

sycl/include/CL/sycl/detail/kernel_program_cache.hpp

@@ -78,6 +82,7 @@ class KernelProgramCache {

  ProgramCacheT MCachedPrograms;
  KernelCacheT MKernelsPerProgramCache;
+  PlatformImplPtr MPlatform;


It seems very strange to have platform object in the cache.
@s-kanaev Do you know if we can do better?

plugin_impl is only kept in Platform object.
To access it, platform_impl has been included here. The platform_impl here is from the context_impl to which the cache belongs.
Will it be better to include a reference to the parent Context? I wasn't able to achieve it in the code. So i decided to pass the platform the context belongs to.

garimagu · 2020-01-23T20:23:25Z

sycl/include/CL/sycl/detail/kernel_program_cache.hpp

@@ -78,6 +82,7 @@ class KernelProgramCache {

  ProgramCacheT MCachedPrograms;
  KernelCacheT MKernelsPerProgramCache;
+  PlatformImplPtr MPlatform;


As far as I understand cache only requires plugin information, not the whole platform. I'd prefer to keep plugin reference/pointer instead of platform.

Yes, only the plugin information is needed. The overall design takes care that only platform_impl owns a shared_ptr to the plugin, and others get a reference to the plugin only through platform_impl class. Storing a reference to plugin_impl has not been done anywhere. So I refrained from storing it here.

Besides., is it even possible to retrieve plugin information out of PI kernel/program object/pointer?

No we cannot access the plugin_information through any PI class objects. Atleast not with the current design.

garimagu · 2020-01-23T20:26:03Z

sycl/include/CL/sycl/detail/kernel_program_cache.hpp

@@ -78,6 +82,7 @@ class KernelProgramCache {

  ProgramCacheT MCachedPrograms;
  KernelCacheT MKernelsPerProgramCache;
+  PlatformImplPtr MPlatform;


Currently no. But in the future yes. The design enables you to connect to multiple Plugins and have multiple platforms from different plugins at the same time.
We have a GlobalPlugin which stores one plugin in use,it is currently used for OpenCL Interoperability constructors only. It will be a bad design to use it here, because we would like to delete that GlobalPlugin in the future.

sycl/include/CL/sycl/detail/pi.hpp

sycl/source/detail/scheduler/commands.cpp

AlexeySachkov · 2020-01-27T12:54:03Z

sycl/include/CL/sycl/buffer.hpp

-    PI_CALL(piMemGetInfo)(detail::pi::cast<detail::RT::PiMem>(MemObject),
-                          CL_MEM_SIZE, sizeof(size_t), &BufSize, nullptr);
+    auto Plugin = detail::getSyclObjImpl(SyclContext)->getPlugin();
+    Plugin.call<detail::PiApiKind::piMemGetInfo>(


What about adding some free functions to avoid explicitly extracting PI Plugin every time? Something like:

template <detail::PiApiKind API, typename... Args> pi_call(const context &SYCLContext, Args...) { auto Plugin = detail::getSYCLObjImpl(SYCLContext)->getPlugin(); Plugin.call<API>(Args...); } // overloads for device, event and other SYCL RT classes // On call site: pi_call<detail::PiApiKind::piMemGetInfo>(SyclContext, /* rest of the args */);

We wanted to avoid making such calls that takes an extra argument beyond the arguments of the function call.
It should look like a function call.
Also, what is the benefit over the existing call format?

@AlexeySachkov any comments?

Also, what is the benefit over the existing call format?

In most cases in this PR, call to a plugin performed only once, so suggested format can save you one line at most of call sites.

We wanted to avoid making such calls that takes an extra argument beyond the arguments of the function call.
It should look like a function call.

I'm not pushing towards my suggestion, this was just an idea. If the intent was to make this call as close as possible to actual call which will be performed under the hood, then let it be. I don't have objections against it

sycl/include/CL/sycl/detail/context_impl.hpp

sycl/include/CL/sycl/intel/function_pointer.hpp

sycl/source/detail/kernel_impl.cpp

sycl/source/detail/memory_manager.cpp

sycl/include/CL/sycl/detail/event_impl.hpp

AlexeySachkov · 2020-01-27T14:25:29Z

sycl/source/detail/scheduler/commands.cpp

+static std::vector<RT::PiEvent>
+getPiEvents(const std::vector<EventImplPtr> &EventImpls) {
+  std::vector<RT::PiEvent> RetPiEvents;
+  for (auto &EventImpl : EventImpls)
+    RetPiEvents.push_back(EventImpl->getHandleRef());
+  return RetPiEvents;
+}


You can replace this with std::transform

In this file, the functionality is called at multiple locations..

@AlexeySachkov

In this file, the functionality is called at multiple locations..

I will leave it up to you, then. Not sure that inserting std::transform into this helper will help a lot, so, you can leave it as is

sycl/include/CL/sycl/detail/plugin.hpp

AlexeySachkov · 2020-01-27T14:33:10Z

sycl/include/CL/sycl/detail/plugin.hpp

+    RT::PiResult Err = call_nocheck<PiApiOffset>(Args...);
+    checkPiResult(Err);
+  }
+  // TODO: Make this private.


Please resolve this TODO. Briefly looking at the PR, there should be no problems with that

@garimagu Please, resolve this one.

sycl/source/detail/pi.cpp

smaslov-intel

This looks good to me, just a few comment on comments.

smaslov-intel · 2020-01-31T00:37:17Z

@smaslov-intel, @kbobrovs, it looks like implementation of plug-in interface matured enough to fill some of design unknowns. Could you update plug-in documentation and replace TBDs with the implemented design, please?

Agree. @garimagu , would you please own that? I think this can go in a separate PR.

romanovvlad · 2020-01-31T16:15:11Z

@garimagu
Could you please resolve all the open comments: either resolve the issue or convince author that it's not an issue.

garimagu

@smaslov-intel, @kbobrovs, it looks like implementation of plug-in interface matured enough to fill some of design unknowns. Could you update plug-in documentation and replace TBDs with the implemented design, please?

Agree. @garimagu , would you please own that? I think this can go in a separate PR.

Sure. I can update the design doc in PR #680 .

garimagu · 2020-01-27T20:32:25Z

sycl/include/CL/sycl/buffer.hpp

-    PI_CALL(piMemGetInfo)(detail::pi::cast<detail::RT::PiMem>(MemObject),
-                          CL_MEM_SIZE, sizeof(size_t), &BufSize, nullptr);
+    auto Plugin = detail::getSyclObjImpl(SyclContext)->getPlugin();
+    Plugin.call<detail::PiApiKind::piMemGetInfo>(


We wanted to avoid making such calls that takes an extra argument beyond the arguments of the function call.
It should look like a function call.
Also, what is the benefit over the existing call format?

sycl/include/CL/sycl/detail/context_impl.hpp

sycl/include/CL/sycl/detail/event_impl.hpp

garimagu · 2020-01-27T22:07:44Z

sycl/include/CL/sycl/detail/kernel_program_cache.hpp

@@ -78,6 +82,7 @@ class KernelProgramCache {

  ProgramCacheT MCachedPrograms;
  KernelCacheT MKernelsPerProgramCache;
+  PlatformImplPtr MPlatform;


I have included the ptr instead of weak_ptr.

sycl/include/CL/sycl/detail/plugin.hpp

sycl/include/CL/sycl/intel/function_pointer.hpp

sycl/source/detail/kernel_impl.cpp

sycl/source/detail/memory_manager.cpp

garimagu · 2020-01-27T23:01:52Z

sycl/source/detail/scheduler/commands.cpp

+static std::vector<RT::PiEvent>
+getPiEvents(const std::vector<EventImplPtr> &EventImpls) {
+  std::vector<RT::PiEvent> RetPiEvents;
+  for (auto &EventImpl : EventImpls)
+    RetPiEvents.push_back(EventImpl->getHandleRef());
+  return RetPiEvents;
+}


In this file, the functionality is called at multiple locations..

garimagu · 2020-01-31T23:23:10Z

@garimagu
Could you please resolve all the open comments: either resolve the issue or convince author that it's not an issue.

Yes. I have resolved the ones I feel were clear enough to be resolved. Please feel free to point out anything I may have missed.
@AlexeySachkov please confirm if any of your concerns haven't been answered.

romanovvlad · 2020-02-03T12:31:07Z

sycl/source/detail/scheduler/commands.cpp

-      PI_CALL(piEnqueueEventsWait)(Queue->getHandleRef(), RawEvents.size(),
-                                   &RawEvents[0], &Event);
+      std::vector<RT::PiEvent> RawEvents = getPiEvents(EventImpls);
+      auto Plugin = Queue->getPlugin();


Minor. Why can't these lines be unified with lines 148 and 149 and moved from if?

I should remove the getPiEvents() call out of the if condition.
But the Plugin needs to be taken from EventImpl and queue in the true and false path respectively.

sycl/source/detail/scheduler/commands.cpp

romanovvlad · 2020-02-03T12:49:18Z

sycl/include/CL/sycl/buffer.hpp

@@ -181,8 +181,10 @@ class buffer {
      : Range{0} {

    size_t BufSize = 0;
-    PI_CALL(piMemGetInfo)(detail::pi::cast<detail::RT::PiMem>(MemObject),
-                          CL_MEM_SIZE, sizeof(size_t), &BufSize, nullptr);
+    auto Plugin = detail::getSyclObjImpl(SyclContext)->getPlugin();


Please, do not use auto in such cases
See: https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable.

hmmm.. Are you asking me to remove auto keyword since the namespace is different and its an implementations specific class in sycl::buffer class?

No, I'm asking you to do this because the type which getPlugin returns is not clear from the context.

Could you please apply to the whole patch. Some of the places where I see similar problem:

3 image_impl.hpp : 237, 355 4 queue_impl.hpp: 91 5 function_pointer.hpp: 83 6 device_impl.hpp: 60, 79, 113 7 kernel_program_cache.cpp: 34, 39 8 memory_manager.cpp: 28, 62, 105, 126, 145, 196, 231, 289, 314, 420, 476, 493, 508, 524, 539 9 platform_impl.cpp: 38, 214 10 program_impl.cpp: 71, 85, 147, 157, 195, 222, 261, 284, 292, 11 ...

I have removed the auto keyword from all locations.

romanovvlad · 2020-02-03T12:50:16Z

I think we are mostly done here. Just a couple of comments.

sycl/include/CL/sycl/detail/plugin.hpp

garimagu · 2020-02-03T19:44:55Z

sycl/source/detail/scheduler/commands.cpp

-      PI_CALL(piEnqueueEventsWait)(Queue->getHandleRef(), RawEvents.size(),
-                                   &RawEvents[0], &Event);
+      std::vector<RT::PiEvent> RawEvents = getPiEvents(EventImpls);
+      auto Plugin = Queue->getPlugin();


I should remove the getPiEvents() call out of the if condition.
But the Plugin needs to be taken from EventImpl and queue in the true and false path respectively.

garimagu · 2020-02-04T01:34:30Z

sycl/include/CL/sycl/buffer.hpp

@@ -181,8 +181,10 @@ class buffer {
      : Range{0} {

    size_t BufSize = 0;
-    PI_CALL(piMemGetInfo)(detail::pi::cast<detail::RT::PiMem>(MemObject),
-                          CL_MEM_SIZE, sizeof(size_t), &BufSize, nullptr);
+    auto Plugin = detail::getSyclObjImpl(SyclContext)->getPlugin();


hmmm.. Are you asking me to remove auto keyword since the namespace is different and its an implementations specific class in sycl::buffer class?

bader · 2020-02-05T08:40:17Z

@garimagu, this change conflicts with #1091. Are you okay if we merge #1091 first and rebase this PR on top of that?

garimagu · 2020-02-05T18:55:13Z

@garimagu, this change conflicts with #1091. Are you okay if we merge #1091 first and rebase this PR on top of that?

@bader . Is it possible to merge this first? This patch is ready to be merged and shouldn't be kept hanging for long. There are rebase conflicts everyday in this PR which require manual changes.
Looking at the changeset at #1091 it looks like it will have merge conflicts with almost all reviews in progress. Moreover, I think it will be easier for it's author than it is for me to resolve merge conflicts with this PR. This PR is way simpler than 1091. If the author of #1091 is ok, I would recommend to merge this PR first.

romanovvlad · 2020-02-06T09:08:24Z

sycl/include/CL/sycl/detail/sycl_mem_obj_t.hpp

@@ -81,6 +83,7 @@ class SYCLMemObjT : public SYCLMemObjI {

  virtual ~SYCLMemObjT() = default;

+  const plugin &getPlugin() const { return MInteropContext->getPlugin(); }


Do you use the method? If no I suggest removing it.
The problem here is that MInteropContext can be/usually it's nullptr. It's not null when user creates a memory object using interop constructor only.

I cannot remove it, since it is used in sycl_mem_obj_t.hpp.
I have added an assert check on the pointer.

…ugin_Impl class. The changeset removes the family of PI_CALL macros, and replaces them with call API in a new plugin_impl class. The object of this class is stored in Platform class. When multiple platforms are associated with the same Plugin, they will have shared_ptr to the same plugin_impl object. Eg: An OpenCLPlugin can have multiple platforms attached to it. To effectively use this change, following additional fetaures/changes were added: - To make access to the plugin_impl class easy, getPlugin() API has been introduced to some *_impl classes, that eventually call the platform_impl->getPlugin() API. It has been included in context_impl, event_impl, kernel_impl, platform_impl, program_impl, queue_impl. Is not present in accessor_impl(Does not call PI), buffer_impl (Needs only in the constructor where context is passed), sampler_impl (Does not have a single context), stream_impl (Does not call PI), usm_impl (Has no member sycl_impl objects.) The API returns const reference to plugin_impl class. - Use of PiApiKind enum to uniquely identify the PI API. - PI API calls are done using : plugin_impl.call, plugin_impl.call_nocheck, plugin_impl.checkPiResult<Exception> - GlobalPlugin is a SharedPtr to a globally available Plugin for the use with Interoperability Constructors. It is used as a shared_ptr to avoid making copies of it. - Changes in *_info structs and get() methods. They now take a const plugin_impl& argument. - To make Pi Api calls by host to check if certain events are finished on a device, the plugin_impl class is needed. At such places, event_impl is passed instead of a PiEvent. Eg: memory_manager.cpp - The use of Global variable has been restricted to only the GlobalPlugin now. Signed-off-by: Garima Gupta <garima.gupta@intel.com>

Signed-off-by: Garima Gupta <garima.gupta@intel.com>

memory leaks. The constructor using PiEvent is like a copy constructor earlier was doing a retain+release. We only need a release when destructor is called. Signed-off-by: Garima Gupta <garima.gupta@intel.com>

Signed-off-by: Garima Gupta <garima.gupta@intel.com>

kenrel_program_cache. Signed-off-by: Garima Gupta <garima.gupta@intel.com>

Removal of platform_impl from kernel_program_cache and addition of its parent (object of context_impl keeping teh instance of kernel_program_cache) context_impl* instead. Signed-off-by: Garima Gupta <garima.gupta@intel.com>

Signed-off-by: Garima Gupta <garima.gupta@intel.com>

Changed auto Plugin to const detail::plugin &Plugin Signed-off-by: Garima Gupta <garima.gupta@intel.com>

Signed-off-by: Garima Gupta <garima.gupta@intel.com>

Addition of a suggested change. Signed-off-by: Garima Gupta <garima.gupta@intel.com>

garimagu · 2020-02-07T00:40:48Z

@romanovvlad Please let me know of any pending issues you see. I will resolve all the conflicts and do one final rebase of the workspace.
If there are no other issues, please approve.

bader · 2020-02-07T13:43:17Z

@garimagu, this change conflicts with #1091. Are you okay if we merge #1091 first and rebase this PR on top of that?

@bader . Is it possible to merge this first? This patch is ready to be merged and shouldn't be kept hanging for long. There are rebase conflicts everyday in this PR which require manual changes.
Looking at the changeset at #1091 it looks like it will have merge conflicts with almost all reviews in progress. Moreover, I think it will be easier for it's author than it is for me to resolve merge conflicts with this PR. This PR is way simpler than 1091. If the author of #1091 is ok, I would recommend to merge this PR first.

@Ruyk, @Alexander-Johnston are you okay if #1030 merged first?

Ruyk · 2020-02-10T09:56:45Z

Yeah thats fine, we are still working on rebasing our PR on top of the latest changes

Running tests with clang-cl would set the proper object extension, but clang and clang++ on Windows would use the wrong .o extension.

garimagu requested review from romanovvlad, smaslov-intel and tovinkere January 21, 2020 01:07

romanovvlad reviewed Jan 21, 2020

View reviewed changes

sycl/source/detail/memory_manager.cpp Show resolved Hide resolved

romanovvlad reviewed Jan 21, 2020

View reviewed changes

sycl/include/CL/sycl/detail/plugin_impl.hpp Outdated Show resolved Hide resolved

s-kanaev reviewed Jan 22, 2020

View reviewed changes

garimagu commented Jan 23, 2020

View reviewed changes

smaslov-intel reviewed Jan 24, 2020

View reviewed changes

sycl/source/detail/scheduler/commands.cpp Outdated Show resolved Hide resolved

romanovvlad reviewed Jan 27, 2020

View reviewed changes

sycl/source/detail/scheduler/commands.cpp Outdated Show resolved Hide resolved

AlexeySachkov reviewed Jan 27, 2020

View reviewed changes

smaslov-intel reviewed Jan 31, 2020

View reviewed changes

sycl/source/detail/pi.cpp Outdated Show resolved Hide resolved

smaslov-intel reviewed Jan 31, 2020

View reviewed changes

sycl/source/detail/pi.cpp Outdated Show resolved Hide resolved

smaslov-intel previously approved these changes Jan 31, 2020

View reviewed changes

bader assigned romanovvlad Jan 31, 2020

garimagu mentioned this pull request Jan 31, 2020

[SYCL][DOC] Update the SYCL Runtime Interface document to give some design updates #680

Merged

garimagu commented Jan 31, 2020

View reviewed changes

garimagu dismissed smaslov-intel’s stale review via 9cc8977 January 31, 2020 22:08

garimagu force-pushed the private/garimagu/piplugin_platformobj branch from 74d3112 to 9cc8977 Compare January 31, 2020 22:08

romanovvlad reviewed Feb 3, 2020

View reviewed changes

romanovvlad requested changes Feb 3, 2020

View reviewed changes

garimagu commented Feb 4, 2020

View reviewed changes

garimagu force-pushed the private/garimagu/piplugin_platformobj branch from 8456386 to ff0911a Compare February 4, 2020 02:00

garimagu dismissed smaslov-intel’s stale review via b602782 February 5, 2020 18:43

garimagu force-pushed the private/garimagu/piplugin_platformobj branch from 1901f8f to b602782 Compare February 5, 2020 18:43

romanovvlad reviewed Feb 6, 2020

View reviewed changes

garimagu added 15 commits February 6, 2020 15:56

[SYC][PI] Fixed a bug in commands.cpp file.

b336fc1

Signed-off-by: Garima Gupta <garima.gupta@intel.com>

[SYCL][PI] Renamed plugin_impl to plugin class.

a38f9eb

Signed-off-by: Garima Gupta <garima.gupta@intel.com>

Changes in commands.cpp file while creation of event_impl to avoid

0d21709

memory leaks. The constructor using PiEvent is like a copy constructor earlier was doing a retain+release. We only need a release when destructor is called. Signed-off-by: Garima Gupta <garima.gupta@intel.com>

Using const reference to pass a shared_ptr instead of passing by value.

ba54c65

Signed-off-by: Garima Gupta <garima.gupta@intel.com>

Include context_impl* ParentContext instead of ptr_platform_impl in

035a29f

kenrel_program_cache. Signed-off-by: Garima Gupta <garima.gupta@intel.com>

Addition of suggested changes.

bd37e6b

Removal of platform_impl from kernel_program_cache and addition of its parent (object of context_impl keeping teh instance of kernel_program_cache) context_impl* instead. Signed-off-by: Garima Gupta <garima.gupta@intel.com>

Addition of suggested changes.

9dd5501

Signed-off-by: Garima Gupta <garima.gupta@intel.com>

The error introduced during rebase/conflict resolution.

37696b9

Signed-off-by: Garima Gupta <garima.gupta@intel.com>

Addition of missing initialization when using std::transform

1dad546

Signed-off-by: Garima Gupta <garima.gupta@intel.com>

Addition of suggested changes.

8bea933

Signed-off-by: Garima Gupta <garima.gupta@intel.com>

Rebase error fix.

8570e84

Signed-off-by: Garima Gupta <garima.gupta@intel.com>

Removed the use of auto keyword.

502557b

Changed auto Plugin to const detail::plugin &Plugin Signed-off-by: Garima Gupta <garima.gupta@intel.com>

rebase errors and issues.

da92ae9

Signed-off-by: Garima Gupta <garima.gupta@intel.com>

Rebase conflict resolution errors and

4bea0ee

Addition of a suggested change. Signed-off-by: Garima Gupta <garima.gupta@intel.com>

garimagu force-pushed the private/garimagu/piplugin_platformobj branch from b602782 to 4bea0ee Compare February 7, 2020 00:37

romanovvlad approved these changes Feb 7, 2020

View reviewed changes

bader merged commit 95652d4 into intel:sycl Feb 10, 2020

Alexander-Johnston mentioned this pull request Feb 20, 2020

[SYCL][CUDA] Initial CUDA backend support #1091

Merged

		@@ -81,6 +83,7 @@ class SYCLMemObjT : public SYCLMemObjI {

		virtual ~SYCLMemObjT() = default;

		const plugin &getPlugin() const { return MInteropContext->getPlugin(); }

[SYCL][PI] Removal of PI_CALL family of macros & implementation of Plugin_impl class #1030

[SYCL][PI] Removal of PI_CALL family of macros & implementation of Plugin_impl class #1030

Uh oh!

Conversation

garimagu commented Jan 21, 2020 • edited by AlexeySachkov Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bader commented Jan 21, 2020

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

garimagu Jan 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

garimagu commented Jan 22, 2020

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

garimagu commented Jan 23, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

garimagu commented Jan 21, 2020 •

edited by AlexeySachkov

Loading

garimagu Jan 24, 2020 •

edited

Loading

romanovvlad Feb 3, 2020 •

edited

Loading