[SYCL][L0] Report root- vs sub- device free memory differently #7789
Conversation
Signed-off-by: Sergey V Maslov <[email protected]>
Signed-off-by: Sergey V Maslov <[email protected]>
Signed-off-by: Sergey V Maslov <[email protected]>
| (*Device->ZeDeviceMemoryProperties.operator->())[I].totalSize; | ||
| for (const auto &ZeDeviceMemoryExtProperty : | ||
| Device->ZeDeviceMemoryProperties->second) { | ||
| GlobalMemSize += ZeDeviceMemoryExtProperty.physicalSize; |
There was a problem hiding this comment.
@smaslov-intel : I guess we are adding here because ZeDevice from line 810 is a a subdevice, right?
There was a problem hiding this comment.
No, Device/ZeDevice may be either sub-device or root- device here.
We are adding because that's what we were doing before and I only change where we take the "total" size from.
So, should we be adding or not?
I assume today there is only one memory, so it's probably not important at the moment.
There was a problem hiding this comment.
physicalSize reported by Core properties for a root device already is the aggregation of the sub-devices. could you confirm whether you are getting the correct numbers here?
There was a problem hiding this comment.
discussed with @smaslov-intel : code is good.
| zes_mem_handle_t ZesMemHandle; | ||
| ZE_CALL(zesDeviceEnumMemoryModules, (ZeDevice, &MemCount, &ZesMemHandle)); | ||
| uint32_t MemCount = 0; | ||
| ZE_CALL(zesDeviceEnumMemoryModules, (ZeDevice, &MemCount, nullptr)); |
There was a problem hiding this comment.
If ZeDevice is sub-device then isn't zesDeviceEnumMemoryModules supposed to return memory modules/handes only for that subdevice? I am trying to understand why we need this if:
https://github.com/intel/llvm/pull/7789/files#diff-15dd1eb076d2164bd9e87d9057f05f652a716498e8cdf5975e564c65309a0985R3195-R3196
There was a problem hiding this comment.
Perhaps you are right, so maybe check for subdeviceID is redundant in the loop (but not wrong).
@jandres742 : would you comment?
There was a problem hiding this comment.
Correct. For a sub-device, we have only one memory module currently, but logic below is scalable if that changes in future.
Line 3189 traverses over the memory modules in the ZexMemHandles vector (it would be N for a parent device with N sub-devices, and 1 for a sub-device that is not further partitioned by L0).
Line 3200 is executed several times only for a root device. For a sub-device, it is executed only if the memory module has the same sub-device id, and currently that would be only once.
There was a problem hiding this comment.
Looks good to me. But this should be merged after @jandres742 approval.
|
+1 |
E2E test: intel/llvm-test-suite#1453
Signed-off-by: Sergey V Maslov [email protected]