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

2022-03-31 Thread Chris Bieneman via Phabricator via cfe-commits
beanz added a comment. I just pushed one more commit rG19054163e11a6632b4973c936e5aa93ec742c866 that further refines the subject specification and covers the merging attributes and conflicting attributes on the same decl. Th

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

2022-03-31 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D122627#3417919 , @beanz wrote: > In D122627#3417557 , @aaron.ballman > wrote: > >> Are you sure that's what you want? This returns true for a static C++ member >> function, fal

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

2022-03-30 Thread Chris Bieneman via Phabricator via cfe-commits
beanz added a comment. In 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 otherwi

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

2022-03-30 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D122627#3417495 , @beanz wrote: > @aaron.ballman I pushed updates in rGff6696c842ba > . > > The one bit we discussed that I didn't do anything for is t

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

2022-03-30 Thread Chris Bieneman via Phabricator via cfe-commits
beanz added a comment. @aaron.ballman I pushed updates in rGff6696c842ba . The one bit we discussed that I didn't do anything for is the template case. Neither clang or the HLSL compiler support parsing Microsoft attributes on

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

2022-03-30 Thread Chris Bieneman via Phabricator via cfe-commits
beanz added inline comments. Comment at: clang/test/SemaHLSL/num_threads.hlsl:48 +#endif + + aaron.ballman wrote: > There are a few test cases that are missing here: > > 1) Writing the attribute on the wrong subject. > 2) Passing no args to the attribute, passin

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

2022-03-30 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/test/SemaHLSL/num_threads.hlsl:48 +#endif + + There are a few test cases that are missing here: 1) Writing the attribute on the wrong subject. 2) Passing no args to the attribute, passing too many args to th

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

2022-03-29 Thread Chris Bieneman via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG94189b42cc51: [HLSL] Fix MSFT Attribute parsing, add numthreads (authored by beanz). Changed prior to commit: https://reviews.llvm.org/D122627?vs=

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

2022-03-29 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Comment at: clang/include/clang/Basic/AttrDocs.td:6377 +are required. The ``X``, ``Y``, and ``Z`` values provided to the attribute +dictate the thread id. Total number of thre

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

2022-03-28 Thread Greg Roth via Phabricator via cfe-commits
pow2clk added a comment. Looks good. I made a comment about the CS 4.0 thing, but that can be addressed when more thorough examination of the requested shader models comes around. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6857 + uint32_t ThreadMax = 1024; + if (SMVersion.g

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

2022-03-28 Thread Chris Bieneman via Phabricator via cfe-commits
beanz created this revision. beanz added reviewers: aaron.ballman, rnk, jdoerfert, MaskRay, rsmith. Herald added a subscriber: StephenFan. Herald added a project: All. beanz requested review of this revision. Herald added a subscriber: sstefan1. Herald added a project: clang. HLSL uses Microsoft-s