This is an archive of the discontinued LLVM Phabricator instance.

[HLSL] Generate buffer subscript operators
ClosedPublic

Authored by beanz on Aug 5 2022, 9:40 AM.

Details

Summary

In HLSL buffer types support array subscripting syntax for loads and
stores. This change fleshes out the subscript operators to become array
accesses on the underlying handle pointer. This will allow LLVM
optimization passes to optimize resource accesses the same way any other
memory access would be optimized.

Diff Detail

Event Timeline

beanz created this revision.Aug 5 2022, 9:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 5 2022, 9:40 AM
Herald added a subscriber: Anastasia. · View Herald Transcript
beanz requested review of this revision.Aug 5 2022, 9:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 5 2022, 9:40 AM
python3kgae added inline comments.Aug 5 2022, 11:31 AM
clang/lib/Sema/HLSLExternalSemaSource.cpp
220

If we reuse this function for StructuredBuffer, then const subscript return a const reference could be better?

clang/test/CodeGenHLSL/buffer-array-operator.hlsl
5

Maybe change In to RWBuffer<float4>
And Out[Idx] = In[Idx].z;
So we also test vector case?

17

Where is the this.addr coming from?

beanz added inline comments.Aug 5 2022, 12:09 PM
clang/lib/Sema/HLSLExternalSemaSource.cpp
220

Why? Since this should get inlined return value optimization should eliminate the redundant copies.

clang/test/CodeGenHLSL/buffer-array-operator.hlsl
5

None of the added code here depends on the template parameter type. Putting a vector in increases the complexity of the output, but I'm unsure it extends the test coverage meaningfully.

17

That is how clang generates stored locations for input parameters.

python3kgae added inline comments.Aug 12 2022, 8:56 AM
clang/test/CodeGenHLSL/buffer-array-operator.hlsl
4

Why add const instead of using Buffer directly?

beanz added inline comments.Aug 12 2022, 5:07 PM
clang/test/CodeGenHLSL/buffer-array-operator.hlsl
4

Making this const forces the const methods to be used. It is just to drive the correct validation and code generation.

python3kgae added inline comments.Aug 12 2022, 5:11 PM
clang/test/CodeGenHLSL/buffer-array-operator.hlsl
4

So maybe we don't need Buffer at all, just use const RWBuffer for read-only usage?

beanz added inline comments.Aug 16 2022, 1:08 PM
clang/test/CodeGenHLSL/buffer-array-operator.hlsl
4

I think there would likely be some code incompatibilities if we alias Buffer to const RWBuffer, but that would be interesting to consider.

beanz updated this revision to Diff 456731.Aug 30 2022, 11:07 AM

Updating based on PR feedback and rebasing.

  • Rebased on main today
  • Made const subscript return type const &
aaron.ballman added inline comments.Aug 30 2022, 12:05 PM
clang/lib/Sema/HLSLExternalSemaSource.cpp
108–113
180

Can you use a static_cast to make this a wee bit safer, or is that not possible?

231–232
241
243

Should this be using a size type instead?

252
254
257
262
266
beanz added inline comments.Aug 30 2022, 12:25 PM
clang/lib/Sema/HLSLExternalSemaSource.cpp
243

As currently implemented it is an unsigned int.

I should include the doc link in the description:
https://docs.microsoft.com/en-us/windows/win32/direct3dhlsl/sm5-object-rwbuffer-operatorindex

Eventually I think we'll need to introduce size types to HLSL, but we currently don't really use them.

beanz updated this revision to Diff 456774.Aug 30 2022, 1:26 PM

Updating based on @aaron.ballman's feedback.

  • Change reinterpret_cast -> static_cast
  • Aaron likes auto more than me... but all in good places :D
aaron.ballman accepted this revision.Sep 2 2022, 10:26 AM

LGTM aside from a nit that was missed. Thanks for switching to static_cast, that makes me happier. :-)

clang/lib/Sema/HLSLExternalSemaSource.cpp
109

Missed from the previous suggestion.

This revision is now accepted and ready to land.Sep 2 2022, 10:26 AM
This revision was automatically updated to reflect the committed changes.