Implement support for groupshared arg attribute #8013

spall · 2025-12-18T18:50:45Z

Implements support for groupshared argument attribute.
Closes #7969

get compiler to error when types aren't exactly the same + arg not groupshared bug fix fix test disallow groupshared param in export/noinline funcs first check if there are attrs remove accidental change tests

update tests to show new mangling

github-actions · 2025-12-18T18:51:52Z

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff 60a1c8568b405bbe9f47ab0431f627ef64988cc4 a6f890ea130e7f749b280123e78ec8ed987e9efa -- lib/HLSL/HLLegalizeParameter.cpp tools/clang/include/clang/AST/Decl.h tools/clang/include/clang/Basic/Specifiers.h tools/clang/lib/AST/HlslTypes.cpp tools/clang/lib/AST/MicrosoftMangle.cpp tools/clang/lib/CodeGen/CGHLSLMS.cpp tools/clang/lib/Sema/SemaDeclAttr.cpp tools/clang/lib/Sema/SemaHLSL.cpp tools/clang/lib/Sema/SemaOverload.cpp

View the diff from clang-format here.

diff --git a/tools/clang/lib/AST/MicrosoftMangle.cpp b/tools/clang/lib/AST/MicrosoftMangle.cpp
index 1532f441..35cbb7cd 100644
--- a/tools/clang/lib/AST/MicrosoftMangle.cpp
+++ b/tools/clang/lib/AST/MicrosoftMangle.cpp
@@ -2035,7 +2035,8 @@ void MicrosoftCXXNameMangler::mangleType(const LValueReferenceType *T,
                                          Qualifiers Quals, SourceRange Range) {
   QualType PointeeType = T->getPointeeType();
   Out << (Quals.hasVolatile() ? 'B' : 'A');
-  if (PointeeType.getQualifiers().getAddressSpace() == hlsl::DXIL::kTGSMAddrSpace)
+  if (PointeeType.getQualifiers().getAddressSpace() ==
+      hlsl::DXIL::kTGSMAddrSpace)
     Out << 'G';
   manglePointerExtQualifiers(Quals, PointeeType);
   mangleType(PointeeType, Range);
diff --git a/tools/clang/lib/Sema/SemaHLSL.cpp b/tools/clang/lib/Sema/SemaHLSL.cpp
index cc27854c..46ab3a51 100644
--- a/tools/clang/lib/Sema/SemaHLSL.cpp
+++ b/tools/clang/lib/Sema/SemaHLSL.cpp
@@ -15771,7 +15771,9 @@ bool Sema::DiagnoseHLSLDecl(Declarator &D, DeclContext *DC, Expr *BitWidth,
         Diag(pAttr->getLoc(), diag::warn_hlsl_groupshared_202x);
       if (isParameter && (usageIn || usageOut)) {
         Diag(pAttr->getLoc(), diag::err_hlsl_varmodifiersna)
-	  << (usageIn && usageOut ? "'inout'" : usageIn ? "'in'" : "'out'")
+            << (usageIn && usageOut ? "'inout'"
+                : usageIn           ? "'in'"
+                                    : "'out'")
             << pAttr->getName() << declarationType;
         result = false;
       }

Check this box to apply formatting changes to this branch.

hekota

Mostly looks good, I just have a few comments and suggestions/nits.

tools/clang/test/SemaHLSL/v202x/groupshared/ScalarTest.hlsl

tools/clang/test/SemaHLSL/v202x/groupshared/NotGroupSharedTest.hlsl

tools/clang/test/SemaHLSL/v202x/groupshared/InOutWarning.hlsl

tools/clang/test/CodeGenHLSL/groupsharedArgs/TemplateTest.hlsl

tools/clang/test/CodeGenHLSL/groupsharedArgs/Overloads.hlsl

tools/clang/test/CodeGenHLSL/groupsharedArgs/ScalarTest.hlsl

tools/clang/lib/AST/HlslTypes.cpp

tools/clang/lib/Sema/SemaHLSL.cpp

tools/clang/include/clang/AST/Decl.h

damyanp · 2026-01-12T23:19:50Z

Please add something to https://github.com/microsoft/DirectXShaderCompiler/blob/main/docs/ReleaseNotes.md as appropriate.

hekota

LGTM!

alsepkow · 2026-01-15T23:56:18Z

docs/ReleaseNotes.md

 - Header file `dxcpix.h` was added to the release package.
 - Moved Linear Algebra (Cooperative Vector) DXIL Opcodes to experimental Shader Model 6.10
- Added support for `long long` and `unsigned long long` compile-time constant evaluation, fixes [#7952](https://github.com/microsoft/DirectXShaderCompiler/issues/7952).
+- Added support for the groupshared attribute for parameters to Shader Model 6.10


I wonder if it would be good to start a habit of linking the PR with the note? I like the idea.

I don't have a preference although I'd be inclined to follow the seeming convention of not including, unless you can find further support for this change.

Some of the items link issues. Some link PRs (the highlights for 1.8.2502). Others do nothing. Just seems random.

Some of the items link issues. Some link PRs (the highlights for 1.8.2502). Others do nothing. Just seems random.

ok i guess the ones i looked at didn't link, but i must have missed some.

alsepkow · 2026-01-16T00:09:08Z

tools/clang/lib/Sema/SemaHLSL.cpp

  const bool IsRCAttr = isa<HLSLReorderCoherentAttr>(A);
+  const bool IsExportAttr = isa<HLSLExportAttr>(A);
+  const bool IsNoInlineAttr = isa<NoInlineAttr>(A);
+  if (IsExportAttr || IsNoInlineAttr) {


Can remove an indentation level by just retrning early if neither condition is true

Suggested change

if (IsExportAttr || IsNoInlineAttr) {

if (!IsExportAttr || !IsNoInlineAttr)

return;

The function doesn't return if this is false.

the if statement should probably not be returning either.

The function doesn't return if this is false.

🤦‍♂️

lib/HLSL/HLLegalizeParameter.cpp

tools/clang/lib/AST/MicrosoftMangle.cpp

inbelic · 2026-01-16T00:02:38Z

tools/clang/lib/Sema/SemaHLSL.cpp

        Diag(pAttr->getLoc(), diag::err_hlsl_usage_not_on_parameter)
            << pAttr->getName() << pAttr->getRange();
        result = false;
+      } else if (isGroupShared) {


Is it guaranteed that groupshared will be processed/specified before inout?

Otherwise, we might need to handle the opposite case above

Looking at the cases for inout and for groupshared I think it is handled regardless of order, but good point and I think this points to a test is missing for this.

inbelic · 2026-01-16T00:08:42Z

tools/clang/lib/Sema/SemaDeclAttr.cpp

    handleSimpleAttribute<NoDuplicateAttr>(S, D, Attr);
    break;
  case AttributeList::AT_NoInline:
+    if (S.LangOpts.HLSL) {


I am not quite sure I understand this change. Would we otherwise not parse the attributes of the parameter without this?

the basic handling wouldn't check if a parameter of a function with the noinline attribute has the groupshared attribute.

tools/clang/include/clang/Basic/DiagnosticSemaKinds.td

spall added 4 commits December 3, 2025 16:37

@llvm-beanz proof of concept

e71c32a

get basic tests working

8b20e9e

get compiler to error when types aren't exactly the same + arg not groupshared bug fix fix test disallow groupshared param in export/noinline funcs first check if there are attrs remove accidental change tests

work on errors and warnings

c3437fa

make 202x to avoid adding warning for groupshared arg

fdca3fc

update tests to show new mangling

github-project-automation bot added this to HLSL Roadmap Dec 18, 2025

github-project-automation bot moved this to New in HLSL Roadmap Dec 18, 2025

spall and others added 3 commits January 5, 2026 09:57

self review

c3cadac

attempt to make clang format happy

8511e4e

chore: autopublish 2026-01-06T21:38:05Z

03e93a7

hekota reviewed Jan 9, 2026

View reviewed changes

spall and others added 3 commits January 12, 2026 14:05

update based on @hekota suggestions

c20b77a

update test comment based on @hekota feedback

3519631

chore: autopublish 2026-01-12T22:28:27Z

210be47

hekota approved these changes Jan 13, 2026

View reviewed changes

replace CHECK-DAGs in test and add comment to ReleaseNotes

85b23c0

alsepkow reviewed Jan 15, 2026

View reviewed changes

alsepkow reviewed Jan 16, 2026

View reviewed changes

inbelic reviewed Jan 16, 2026

View reviewed changes

spall and others added 2 commits January 16, 2026 16:28

respond to pr feedback

a6f890e

chore: autopublish 2026-01-17T00:31:02Z

c9f1ab1

	if (IsExportAttr \|\| IsNoInlineAttr) {
	if (!IsExportAttr \|\| !IsNoInlineAttr)
	return;

Implement support for groupshared arg attribute #8013

Are you sure you want to change the base?

Implement support for groupshared arg attribute #8013

Conversation

spall commented Dec 18, 2025

Uh oh!

github-actions bot commented Dec 18, 2025 • edited by spall Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hekota left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

damyanp commented Jan 12, 2026

Uh oh!

hekota left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spall Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

github-actions bot commented Dec 18, 2025 •

edited by spall

Loading

spall Jan 16, 2026 •

edited

Loading