This is an archive of the discontinued LLVM Phabricator instance.

[DirectX backend] support ConstantBuffer to DXILResource.h
ClosedPublic

Authored by python3kgae on Oct 15 2022, 7:33 PM.

Details

Summary

class ConstantBuffer is added to save information for cbuffer.
Also add CBufferDataLayout to calculate the size for cbuffer.

Now always use legacy cbuffer layout.
https://reviews.llvm.org/D134998 will add control to disable legacy cbuffer layout.

Diff Detail

Event Timeline

python3kgae created this revision.Oct 15 2022, 7:33 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 15 2022, 7:33 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
python3kgae requested review of this revision.Oct 15 2022, 7:33 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 15 2022, 7:33 PM

arc diff git merge-base HEAD origin --update D136031 to fix the apply patch issue

Herald added a project: Restricted Project. · View Herald TranscriptOct 15 2022, 7:37 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Rebase and remove the counter in resource metadata to match clang change.

beanz added a comment.Jan 3 2023, 12:07 PM

It is probably worth adding some unit tests to test the CBufferDataLayout class.

I think the meat of this change is fine. This code mixes unsigned and uint32_t interchangeably. They aren't required by the language to be the same.

I have a general preference toward uint32_t which is explicitly sized, but we should match existing interfaces where appropriate.

For example, we should also probably be using llvm::TypeSize where appropriate to match llvm::DataLayout

Use llvm::TypeSize and uint32_t instead of unsigned.

Add unit test.

beanz accepted this revision.Jan 12 2023, 6:40 AM

LGTM

This revision is now accepted and ready to land.Jan 12 2023, 6:40 AM

Fix test fail after rebase.

This revision was landed with ongoing or failed builds.Jan 12 2023, 10:42 AM
This revision was automatically updated to reflect the committed changes.
beanz added a comment.Jan 18 2023, 9:49 AM

@python3kgae, this change introduced a bunch of warning spew because it is using an API that was deprecated shortly before the change merged. Can you please address this?

@python3kgae, this change introduced a bunch of warning spew because it is using an API that was deprecated shortly before the change merged. Can you please address this?

Sure.
I'll fix it.