[SYCL] Cache implicit local arg position info #18654
Conversation
sergey-semenov
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
sergey-semenov
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
sergey-semenov
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
| }; | ||
|
|
||
| if (!KernelNameBasedCachePtr) | ||
| return getLocalArgPos(); |
There was a problem hiding this comment.
As I understand, we keep this for backward compatibility? If so, we should keep it under the #ifndef __INTEL_PREVIEW_BREAKING_CHANGES so that we can remove this code in the next window for the breaking changes.
| std::optional<std::optional<int>> &ImplicitLocalArgPos = | ||
| KernelNameBasedCachePtr->ImplicitLocalArgPos; | ||
| if (!ImplicitLocalArgPos.has_value()) { | ||
| ImplicitLocalArgPos = getLocalArgPos(); |
There was a problem hiding this comment.
If the kernel does not have an implicit arg, the m_KernelImplicitLocalArgPos does not contain an entry for such kernel, and the getLocalArgPos function returns an std::optional that does not contain the value. So the if (!ImplicitLocalArgPos.has_value()) branch is always taken in that case. So I think we need to populate the KernelNameBasedCachePtr->ImplicitLocalArgPos only once in the ProgramManager::addImage function when we call the ProgramManager::cacheKernelImplicitLocalArg.
There was a problem hiding this comment.
Maybe we can initialize it when an instance of the KernelNameBasedCacheT is created? What do you think?
No description provided.