llvm / llvm/llvm-project

[HLSL] Local resource binding-ambiguity warning only tracks plain variable assignments

Open
#216,200 0 comments 0 reactions 0 assignees View on GitHub
clang:diagnostics HLSL
Dominant language
LLVM
Stars
40.5k
Forks
18.7k
PR merge metrics
PR metrics pending

Description

`-Whlsl-explicit-binding` diagnoses binding-ambiguous reassignment of a local resource, but the tracking only recognizes assignments whose LHS is a plain variable reference. Every other lvalue form is silently untracked, so equally ambiguous code compiles without a diagnostic.

## Root cause

`SemaHLSL::CheckResourceBinOp` (`clang/lib/Sema/SemaHLSL.cpp`) unwraps `ArraySubscriptExpr` and then requires the LHS to be a `DeclRefExpr` naming a `VarDecl` before calling `trackLocalResource`. Anything else falls off the end of the function and is never tracked.

`SemaHLSL::inferGlobalBinding` handles `ConditionalOperator` and `ArraySubscriptExpr` on the RHS, but **neither** side handles `MemberExpr`.

## Reproducer

```hlsl
RWByteAddressBuffer G0 : register(u0);
RWByteAddressBuffer G1 : register(u1);
struct S { RWByteAddressBuffer Buf; };

void control(uint i) { // warns (correct)
RWByteAddressBuffer B = G0;
B = G1;
B.Store(i, 1);
}
void member_lhs(uint i) { // silent
S s;
s.Buf = G0;
s.Buf = G1;
s.Buf.Store(i, 1);
}
void ternary_lhs(uint i, bool c) { // silent
RWByteAddressBuffer A = G0;
RWByteAddressBuffer B = G1;
(c ? A : B) = G0;
A.Store(i, 1);
B.Store(i, 2);
}
void member_rhs(uint i) { // silent
S s;
s.Buf = G0;
RWByteAddressBuffer B = s.Buf;
B = G1;
B.Store(i, 1);
}
```

Compiled with `clang -cc1 -std=hlsl202x -finclude-default-header -triple dxil-pc-shadermodel6.6-library -fsyntax-only`, only `control` is diagnosed:

```
warning: assignment of 'G1' to local resource 'B' is not to the same unique global resource [-Whlsl-explicit-binding]
1 warning generated.
```

The other three are equally ambiguous. `member_lhs` and `ternary_lhs` go on to hit the DirectX backend error *"Resource access is not guaranteed to map to a unique global resource"*, so the ambiguity is real and merely undetected at Sema.

## Expected behavior

All four functions should produce `-Whlsl-explicit-binding`.

## Implementation note

`Assigns` is keyed by `VarDecl *`. Covering `MemberExpr` requires a key that can represent a member path (e.g. base decl plus field chain), not just a decl. For a `ConditionalOperator` LHS, both arms are potential assignment targets and presumably both need tracking.

## Tests

These two tests were written for these cases and removed from #190037 in review, on the basis that a test asserting the absence of the diagnostic entrenches the bug. They should be re-added, expecting the new diagnostic, when this is fixed. Note the expected wording is not obvious: the current message interpolates a `VarDecl`, and there is no `VarDecl` for `s.Buf` or `(c ? A : B)`.

`clang/test/SemaHLSL/Resources/Local-Resources/local_resource_ternary_lvalue_ambiguous.hlsl`:

```hlsl
// RUN: %clang_cc1 -std=hlsl202x -finclude-default-header -triple dxil-pc-shadermodel6.6-compute %s -emit-llvm -o - -verify

// expected-no-diagnostics
RWByteAddressBuffer GBuf0 : register(u0);
RWByteAddressBuffer GBuf1 : register(u1);

[numthreads(1,1,1)]
void main(uint3 Tid : SV_DispatchThreadID) {
RWByteAddressBuffer A = GBuf0;
RWByteAddressBuffer B = GBuf1;
bool Cond = Tid.x > 0;
(Cond ? A : B) = GBuf0;
A.Store(Tid.x * 4, 1);
B.Store(Tid.x * 4, 2);
}
```

`clang/test/CodeGenHLSL/resources/Local-Resources/local_resource_struct_member_reassign_ambiguous.hlsl` (this one additionally CHECKed that both ambiguous bindings materialize, which is not something we want to assert; only its source is worth keeping):

```hlsl
// RUN: %clang_cc1 -std=hlsl202x -finclude-default-header -triple dxil-pc-shadermodel6.6-compute %s -emit-llvm -O1 -o - | FileCheck %s

// TODO: This test ought to produce an ambiguity warning, but currently does not.
// This test should move to SemaHLSL and check for the warning.

RWByteAddressBuffer GBuf0 : register(u0);
RWByteAddressBuffer GBuf1 : register(u1);

struct ResHolder { RWByteAddressBuffer Buf; };

[numthreads(4,1,1)]
void main(uint Tid : SV_GroupThreadID) {
ResHolder H;
H.Buf = GBuf0;
if (Tid)
H.Buf = GBuf1;
H.Buf.Store(0, 42);
}

// CHECK-LABEL: define {{.*}}@main(
// Binding for GBuf0 (register(u0, space0)) is emitted.
// CHECK-DAG: call {{.*}}handlefrombinding{{.*}}(i32 0, i32 0,
// Binding for GBuf1 (register(u1, space0)) is emitted.
// CHECK-DAG: call {{.*}}handlefrombinding{{.*}}(i32 0, i32 1,
```

Contributor guide

Open the contributing guide

Research direction

Start in clang/lib/Sema/SemaHLSL.cpp, especially SemaHLSL::CheckResourceBinOp and inferGlobalBinding, to trace how MemberExpr and ConditionalOperator lvalues and rvalues are handled and how Assigns is keyed. Re-add or adapt the sources from clang/test/SemaHLSL/Resources/Local-Resources/local_resource_ternary_lvalue_ambiguous.hlsl and clang/test/CodeGenHLSL/resources/Local-Resources/local_resource_struct_member_reassign_ambiguous.hlsl as SemaHLSL tests. Run the named tests and verify the four reproducer cases receive -Whlsl-explicit-binding diagnostics.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
compilers
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.