[PATCH] D158820: [Sema][HLSL] Fix naming of anyhit/closesthit shaders

Chris Bieneman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 25 11:49:31 PDT 2023


beanz added inline comments.


================
Comment at: clang/test/SemaHLSL/shader_type_attr.hlsl:30
 
 // expected-error at +1 {{'shader' attribute parameters do not match the previous declaration}}
+[shader("pixel")]
----------------
bob80905 wrote:
> bogner wrote:
> > bob80905 wrote:
> > > I don't think we should expect this error, given that there is no previous declaration for doubledUp(). Maybe something else like %0 and %1 are incompatible attributes? 
> > I don't necessarily disagree, but if we're going to change that I think it should be in a different change than this one.
> I'm also curious, why is the change from compute to pixel necessary here?
I suspect this change is just to prevent the presence of the `compute` annotation from triggering the `missing numthreads` error.


================
Comment at: clang/test/SemaHLSL/shader_type_attr.hlsl:61
 // CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}} <line:61:2, col:18> Compute
-[shader("compute")]
+[shader("compute"), numthreads(8,1,1)]
 int entry() {
----------------
Oh interesting! DXC doesn't actually support this syntax, but I think it is worth supporting in Clang.
Should we also check for the numthreads attribute?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158820/new/

https://reviews.llvm.org/D158820



More information about the cfe-commits mailing list