Page MenuHome

Cycles: Distinguish Apple GPUs by core count
ClosedPublic

Authored by Michael Jones (michael_jones) on Jun 21 2022, 6:18 PM.

Details

Summary

This patch suffixes Apple GPU device names with (GPU - # cores) so that variant GPUs with the same chipset can be distinguished. Currently benchmark scores for these M1 family GPUs are being incorrectly merged:

  • M1: 7 or 8 cores
  • M1 Pro: 14 or 16 cores
  • M1 Max: 24 or 32 cores
  • M1 Ultra: 48 or 64 cores

Diff Detail

Repository
rB Blender
Branch
arcpatch-D15257 (branched from master)
Build Status
Buildable 22632
Build 22632: arc lint + arc unit

Event Timeline

Michael Jones (michael_jones) requested review of this revision.Jun 21 2022, 6:18 PM
Michael Jones (michael_jones) created this revision.
This revision is now accepted and ready to land.Jun 21 2022, 6:31 PM
Sergey Sharybin (sergey) requested changes to this revision.Jun 22 2022, 10:10 AM
Sergey Sharybin (sergey) added inline comments.
intern/cycles/device/metal/device_impl.mm
284–289

Having timing information is fine, but such changes should be sneaked into a change which advertises self as a change in device naming.

630

How is this related to the device naming changes?

intern/cycles/util/system.cpp
77 ↗(On Diff #52735)

This change technically breaks scripts compatibility with an immediate outcome that it will have a direct affect on the opendata.blender.org. For example the current CPU name 'Intel(R) Xeon(R) W-3245 CPU @ 3.20GHz is changed to 'Intel(R) Xeon(R) W-3245 CPU @ 3.20GHz (CPU).

Changes in the CPU naming is also not covered in the description of the change. So I can not give any suggestions about possible non-breaking changes solution here as I am not sure what was the problem this change aimed to fix.

This revision now requires changes to proceed.Jun 22 2022, 10:10 AM
  • Revert unrelated changes
Michael Jones (michael_jones) marked 2 inline comments as done.
  • Revert change which appends " (CPU)" to CPU device string
Michael Jones (michael_jones) marked an inline comment as done.Jun 22 2022, 11:52 AM
Michael Jones (michael_jones) added inline comments.
intern/cycles/device/metal/device_impl.mm
284–289

Split out into D15268

630

Split out into D15267

intern/cycles/util/system.cpp
77 ↗(On Diff #52735)

I have reverted this change. The intention was to clarify the device names in the device preferences panel. Without this the device names will be, e.g:

  • Apple M1 Max (GPU - 32 cores)
  • Apple M1 Max (for the CPU device)

I still think it would be nicer to add the clarification for the CPU, but we might need to do this closer to the UI code so as not to break anything that relies on string matching.

Michael Jones (michael_jones) marked an inline comment as done.
  • Get gpu-core-count from specific registry entry instead of doing a scan

Thanks for splitting the change to a separate patches!

Think patch seems good to go as it is now.

For the M1 CPU: we can indeed alter the label in the interface, avoiding changes on the API side. Not sure whether showing the exact CPU name is the best way there even: what matters is that you enable CPU support so that work is split on all the compute devices. Exact frequency and brand does not add that much information there IMO, that's what belongs to the system-info.

This revision is now accepted and ready to land.Jun 22 2022, 2:09 PM