[PATCH] D122627: [HLSL] Fix MSFT Attribute parsing, add numthreads

Thu Mar 31 04:06:23 PDT 2022

aaron.ballman added a comment.

In D122627#3417919 <https://reviews.llvm.org/D122627#3417919>, @beanz wrote:

> In D122627#3417557 <https://reviews.llvm.org/D122627#3417557>, @aaron.ballman wrote:
>
>> Are you sure that's what you want? This returns true for a static C++ member function, false for a static free function, and false for within an unnamed namespace, and true otherwise.
>
> You're right this isn't quite right, but getting closer... HLSL doesn't support unnamed namespaces, and we only support static free functions by accident... (the current compiler ignores static on free functions).
>
> This actually revealed some gaps in the documentation for HLSL, I've gone back and gotten feedback from my team's HLSL expert and I think I've got the right set of constraints for where this can be applied now (clang will even have this better than the HLSL compiler).

SGTM, thanks for digging into it!

>> Also, I didn't see any new test coverage for function merging behavior.
>
> Doh! I knew I was forgetting something. Juggling too many balls today. I'll get that covered too!

No worries, thanks for following up on it. :-)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122627