[DeivceASAN] Make ShadowMemory one instance per type #2585
Conversation
yingcong-wu
force-pushed
the
yc-test-main/0120-on-shadow-per-type
branch
from
24f239c to
c2aa7cd
Compare
|
Hi @unified-runtime-maintain, this patch is targeting v0.11, please help tag this PR. |
| @@ -36,8 +36,7 @@ AsanInterceptor::~AsanInterceptor() { | |||
| // We must release these objects before releasing adapters, since | |||
| // they may use the adapter in their destructor | |||
| for (const auto &[_, DeviceInfo] : m_DeviceMap) { | |||
| [[maybe_unused]] auto URes = DeviceInfo->Shadow->Destory(); | |||
| assert(URes == UR_RESULT_SUCCESS); | |||
| DeviceInfo->Shadow = nullptr; | |||
There was a problem hiding this comment.
clear the "m_DeviceMap" directly?
There was a problem hiding this comment.
Yes, that should also work. But we aim to do minimum change here, so I would respectful reject this advice. We would have follow-up PR to do a better job at the refactoring.
typo: too late |
Updated, thanks. |
|
Thank you @pbalcer , we will hold this PR for now to discuss an issue during our meeting tomorrow. Will mark this as ready when there is a conclusion. |
|
Some CI tasks fail at apt install stage. Suspecting infrastructure problem. |
Yeah, looks like Intel infra issue. Seems to be resolved now. |
|
Hi @pbalcer , CI tasks are all good now. Could you please help merge this PR? |
UR PR: oneapi-src/unified-runtime#2585 --------- Co-authored-by: Kenneth Benzie (Benie) <[email protected]>
…-on-shadow-per-type [DeivceASAN] Make ShadowMemory one instance per type
Previously we had one
ShadowMemoryfor everyDeviceInfo. That is causing problem because it's release process is called too late. And only the firstShadowMemorywould do the actual shadow memory bookkeeping.This is a quick fix but also is heading in the direction we plan, which is one
ShadowMemoryfor each device type. This relies on the fact that for the virtual memory with L0, it can access the address from different device and within different context with newer driver. And as for opencl CPU device, since it is host memory, it does not have to worry about this issue.We will purpose PR later to do a better refactor, but for now we are trying to fix the issue with minimum changes to avoid any possible regression.
Intel/llvm PR: intel/llvm#16687