Page MenuHome

Cycles: Metal readiness: Specify DeviceQueue::enqueue arg types
ClosedPublic

Authored by Michael Jones (michael_jones) on Nov 24 2021, 9:09 PM.

Details

Summary

This patch adds new arg-type parameters to DeviceQueue::enqueue and its overrides. This is in preparation for the Metal backend which needs this information for correct argument encoding.

Ref T92212

Diff Detail

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

Event Timeline

Michael Jones (michael_jones) requested review of this revision.Nov 24 2021, 9:09 PM
Michael Jones (michael_jones) created this revision.
Brecht Van Lommel (brecht) requested changes to this revision.Nov 26 2021, 6:36 PM

What's a bit fragile here is that this information is only used by the Metal backend, so it might get out of sync.

What do you think about something like this?

struct DeviceKernelArguments {
  enum Type {
    POINTER,
    INT32,
    ...
  };

  const int MAX_ARGS = 16;
  Type types[MAX_ARGS];
  void *values[MAX_ARGS];
  size_t size = 0;

   void add(device_ptr *value) { add(POINTER, value); };
   void add(int32_t *value) { add(INT32, value); };
   ...
   void add(Type type, void *value) { types[size] = type; values[size] = value; size++; }
};
intern/cycles/device/queue.h
47

Rename to ARRAY_SIZE (same as in Blender) and move to util/defines.h.

This revision now requires changes to proceed.Nov 26 2021, 6:36 PM

What do you think about something like this?

struct DeviceKernelArguments {
  enum Type {
    POINTER,
    INT32,
    ...
  };

  const int MAX_ARGS = 16;
  Type types[MAX_ARGS];
  void *values[MAX_ARGS];
  size_t size = 0;

   void add(device_ptr *value) { add(POINTER, value); };
   void add(int32_t *value) { add(INT32, value); };
   ...
   void add(Type type, void *value) { types[size] = type; values[size] = value; size++; }
};

I really like this idea. How about buffering the args directly into the DeviceQueue base class, and then enqueue would just act like a flush?

  • Ensure enqueue arg type correctness via DeviceKernelArguments struct
Michael Jones (michael_jones) marked an inline comment as done.Nov 29 2021, 1:30 PM
Brecht Van Lommel (brecht) requested changes to this revision.Nov 29 2021, 3:44 PM

Looks fine, just minor tweak needed.

intern/cycles/device/cuda/device_impl.cpp
480–482

Change NULL to 0.

intern/cycles/device/cuda/queue.cpp
138

Use const_cast<void**>(args.values).

intern/cycles/device/hip/queue.cpp
137

Use const_cast<void**>(args.values).

intern/cycles/integrator/path_trace_work_gpu.cpp
408

Change NULL to 0, this is giving me a build error on Linux/clang.

This revision now requires changes to proceed.Nov 29 2021, 3:44 PM
  • Linux build fix + explicit const casting
Michael Jones (michael_jones) marked 4 inline comments as done.Nov 29 2021, 3:51 PM
This revision is now accepted and ready to land.Nov 29 2021, 3:54 PM