Introduce a new Buffer type with DX,VK,MTL implementations. by manon-traverse · Pull Request #976 · llvm/offload-test-suite

manon-traverse · 2026-03-16T13:49:23Z

A first small step towards creating a Render Backend API layer on top of DX12, Vulkan, and Metal.

This introduces a new Buffer type with an implementation for each API as well as a function on the Device class to create one.

The change is smaller than it looks due to a naming conflict that needed to be resolved. The type previously called Buffer has been renamed to CPUBuffer.

manon-traverse · 2026-03-16T15:59:26Z

lib/API/VK/Device.cpp

+    if (auto Err = initializeLogicalDevice())
+      return Err;


Waaah, I just noticed this. Need to restructure this. Don't want device initialization in buffer creation.

I am solving this issue in: #987

scuff3drook · 2026-03-16T19:43:16Z

include/API/Device.h

+
+struct BufferCreateDesc {
+  MemoryLocation Location;
+  BufferUsage Usage;


I don't think I see this Usage flag used anywhere. Is the idea that buffer usage describes the buffer as CBV/UAV/SRV (in DX12 parlance)?

The Usage tag will later be used to specify the type of buffer (Storage, AccelerationStructure, Constant, etc). I included it here so that the field should be filled in from the start. Perhaps we should apply a constructor on this type to enforce filling in all fields?

scuff3drook · 2026-03-16T19:46:03Z

include/API/Device.h

 };

+enum class MemoryLocation {
+  GpuOnly,


This may be confusing if later on you want to add CPU-visible memory which is still GPU local (ReBAR).

I wonder if the memory locations should just be GpuLocal or HostLocal and then the usage of Readback, Upload, ShaderMutable, or ShaderVisible then dictates the heap/memory-type in conjunction with "MemoryLocation"

In our closed-source render framework, we use CpuToGpu to express that the resource has to be HOST visible and won't be read from. Whether that resource is actually in GPU Local ReBAR, some form of shared memory or on HOST. I am basically using that as a starting point as it has proven to have worked so far, but I am open for suggestion and changes.

That's why the terms GpuLocal and HostLocal may not fit as well, since it's a detail that the rendering backend should deal with and not necessarily the cross-platform rendering code.

Sure, but the enum name here is MemoryLocation after all. I think hiding that as an implementation detail totally makes sense, but it may make sense for the type name to then indicate as such. Personally, if the goal is to expose strictly semantics to the user of this API, and the specific memory location is an implementation detail, I would probably opt to just expose the BufferUsage flag alone.

scuff3drook · 2026-03-16T19:46:51Z

lib/API/DX/Device.cpp

+  createBuffer(llvm::StringRef Name, BufferCreateDesc &Desc,
+               size_t SizeInBytes) override {
+
+    D3D12_HEAP_TYPE HeapType = 0;


Heap type seems unused

scuff3drook · 2026-03-16T19:47:21Z

lib/API/DX/Device.cpp

+    }
+
+    const D3D12_RESOURCE_FLAGS Flags =
+        D3D12_RESOURCE_FLAG_ALLOW_UNORDERED_ACCESS;


These flags aren't yet used, but I'm assuming UAV access should be conditional depending on usage

On Buffers UAV access is kind-of the default. There is no special swizzling or compression like on textures. There may be some exceptions when it comes to acceleration structures that according to the spec shouldn't be UAV. However, I would like to solve that problem once we reach that point.

Yea that's definitely true (re: difference between buffers and textures), although the semantic difference for buffers is often still meaningful in most engines -- you wouldn't expect to create a UAV descriptor for a read-only buffer for example (unless you live in a BDA-only world)

llvm-beanz

A couple comments. As a meta-comment, we do use clang-tidy to enforce consistent style and coding standards. That is run with -Werror in our bots, so some of the CI failures you're seeing as build failures are Clang-Tidy failures.

Our ReadMe documents setting up Clang-Tidy once you have it installed: https://github.com/llvm/offload-test-suite?tab=readme-ov-file#enabling-clang-tidy

llvm-beanz · 2026-03-17T16:05:46Z

lib/API/DX/Device.cpp

+class DXBuffer : public offloadtest::Buffer {
+public:
+  ComPtr<ID3D12Resource> Buffer;
+  std::string Name;


Should we put the Name and SizeInBytes in the base class? Seems like all the sub-classes have those fields.

I am not sure! I am a big fan of using Pure Virtual Classes and only doing functionality inheritance without any data inheritance, much like traits in Rust. What is the LLVM approach to this?

llvm-beanz · 2026-03-17T16:09:39Z

lib/API/VK/Device.cpp

+    default:
+      assert(false && "MemoryLocation variant not handled");
+      break;
+    }
+


If your switch covers all cases the compiler will emit a diagnostic that we'll catch with -Werror in CI, but that only works if you don't have the default case.

Suggested change

default:

assert(false && "MemoryLocation variant not handled");

break;

}

}

assert(MemFlags != 0 && "MemoryLocation variant must not have set flags correctly");

Yes I found that out this morning! Just getting back into C++ after having used Rust for so long.

manon-traverse · 2026-03-17T16:31:08Z

A couple comments. As a meta-comment, we do use clang-tidy to enforce consistent style and coding standards. That is run with -Werror in our bots, so some of the CI failures you're seeing as build failures are Clang-Tidy failures.

Our ReadMe documents setting up Clang-Tidy once you have it installed: https://github.com/llvm/offload-test-suite?tab=readme-ov-file#enabling-clang-tidy

Yes I saw something along the lines of that. I'll check it out! <3

I have some local changes to the README with steps and pointers that I found difficult to figure out to improve the onboarding in the future.

manon-traverse commented Mar 16, 2026

View reviewed changes

scuff3drook reviewed Mar 16, 2026

View reviewed changes

llvm-beanz reviewed Mar 17, 2026

View reviewed changes

manon-traverse added 8 commits March 18, 2026 13:54

Clean-up code to pass clang-tidy.

df5c8c5

Restructure Device and Queue creation.

acd5ed2

Introduce a new Buffer type with DX,VK,MTL implementations.

6e50430

Add heap type selection to DX12 and clean up.

da6f870

Remove redundant comments.

905c290

Use the selected HeapType.

5e8b752

Remove unnecessary default labels.

2042862

Fix up after rebase.

65a94ab

manon-traverse force-pushed the render-backend-api-buffer branch from 7120c5e to 65a94ab Compare March 18, 2026 13:45

Conversation

manon-traverse commented Mar 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

scuff3drook Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

llvm-beanz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

manon-traverse commented Mar 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

scuff3drook Mar 18, 2026 •

edited

Loading