Skip to content

[Resource] Add tests for resource access in control flow#1374

Open
inbelic wants to merge 11 commits into
llvm:mainfrom
inbelic:inbelic/resource-access
Open

[Resource] Add tests for resource access in control flow#1374
inbelic wants to merge 11 commits into
llvm:mainfrom
inbelic:inbelic/resource-access

Conversation

@inbelic

@inbelic inbelic commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

This pr adds test coverage for resource access patterns that are potential optimizations in modern llvm. These optimizations can create illegal looking resource accesses in clang. DXC does not represent resource access through a regular store/load and will perform any such optimizations.

Optimizations considered:

  • GVN sink of load/store
  • SimplifyCFG sink of load/store
  • InstCombine phi sink of load
  • LICM loop invariant sink

These are considered for [non-]dynamic resources and resources that result in a gep of a struct member.

Assisted by: Claude Opus 4.8

@inbelic inbelic added test-all When applied to a PR this will opt-in to additional pre-merge test configurations.. and removed test-all When applied to a PR this will opt-in to additional pre-merge test configurations.. labels Jul 15, 2026
@inbelic
inbelic force-pushed the inbelic/resource-access branch from 8f6eb59 to 176e283 Compare July 15, 2026 23:38
@inbelic
inbelic marked this pull request as ready for review July 21, 2026 23:18
#--- source.hlsl

// Dynamic resource: Bufs[I] is indexed by the loop induction variable, so each
// iteration selects a different resource. LICM cannot hoist the load, and the

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Co-pilot suggested the filename should be 'licm-no-hoist-load-array.test'.
I agree, Thanks co-pilot!


Results:
- Result: BufOut
Rule: BufferFloatULP

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be an exact match instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, changed the type to uint to make it easier and to match exactly

RWStructuredBuffer<float> LocalBuf = Bufs[0];

if ((TID & 1u) == Condition)
LocalBuf = Bufs[1];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might this be a scenario where we use NonUniformResourceIndex ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants