Implementation of GroupSharedLimit to allow increased GroupSharedMemory #7871

JoeCitizen · 2025-11-04T22:42:51Z

Add a new HLSL attribute for Compute, Amp and Mesh shaders: GroupSharedLimit.

This is used to limit the amount of group shared memory a shader is allowed to statically declare, and validation will fail if the limit is exceeded.

There is no upper limit on this attribute, and it is expected that shader writers set the limit as the lowest common denominator for their target hardware and software use case (typically 48k or 64k for modern GPUs).

If no attribute is declared the existing 32k limit is used to be compatible with existing shaders.

Extends the PSV structures to include the selected limit so that runtime validation can reject the shader if it exceeds the device support.

github-actions · 2025-11-04T22:43:55Z

✅ With the latest revision this PR passed the C/C++ code formatter.

damyanp

Some superficial things that'll need to be addressed at some point, I'm afraid I can't really comment on the actual details of how this works.

I suspect that the whole thing needs to be run through clang-format as well.

damyanp · 2025-11-05T00:23:01Z

lib/DxilContainer/DxilPipelineStateValidation.cpp

  }
 }

+void hlsl::SetShaderProps(PSVRuntimeInfo4 *pInfo4, const DxilModule &DM) {


Suggested change

void hlsl::SetShaderProps(PSVRuntimeInfo4 *pInfo4, const DxilModule &DM) {

void hlsl::SetShaderProps(PSVRuntimeInfo4 *Info4, const DxilModule &DM) {

For new code we generally follow the LLVM Coding Standards. This includes no-p-prefix.

damyanp · 2025-11-05T00:23:47Z

lib/DxilContainer/DxilPipelineStateValidation.cpp

                               PSVRuntimeInfo1 *pInfo1, PSVRuntimeInfo2 *pInfo2,
-                               PSVRuntimeInfo3 *pInfo3, uint8_t ShaderKind,
-                               const char *EntryName, const char *Comment) {
+                               PSVRuntimeInfo3 *pInfo3, PSVRuntimeInfo4 *pInfo4,


IMO in this case following the pInfoN pattern is appropriate.

damyanp · 2025-11-05T00:27:04Z

lib/DxilValidation/DxilValidation.cpp

+      if (SpecifiedTGSMSize > 0) {
+      MaxSize = SpecifiedTGSMSize;
+      }


Suggested change

if (SpecifiedTGSMSize > 0) {

MaxSize = SpecifiedTGSMSize;

}

if (SpecifiedTGSMSize > 0)

MaxSize = SpecifiedTGSMSize;

LLVM coding standards say to omit braces here.

Something's also up with the formatting. Did the format-checker spot it? Anyway, clang-format should fix this for you.

tex3d · 2026-01-06T00:48:47Z

lib/DXIL/DxilMetadataHelper.cpp


+    case DxilMDHelper::kDxilGroupSharedLimitTag: {
+      DXASSERT(props.IsCS(), "else invalid shader kind");
+      props.groupSharedLimitBytes = ConstMDToUint32(MDO);


We should set m_bExtraMetadata if shader model is not 6.10 or higher. This is how validation catches newer metadata used in prior shader models where it's unsupported.

if (!m_pSM->IsSMAtLeast(6, 10)) m_bExtraMetadata = true;

tex3d · 2026-01-06T01:37:58Z

lib/DxilValidation/DxilContainerValidation.cpp

  hlsl::SetShaderProps((PSVRuntimeInfo0 *)&DMPSV, DM);
  hlsl::SetShaderProps((PSVRuntimeInfo1 *)&DMPSV, DM);
  hlsl::SetShaderProps((PSVRuntimeInfo2 *)&DMPSV, DM);
+  hlsl::SetShaderProps((PSVRuntimeInfo4 *)&DMPSV, DM);


So, part of the purpose of setting the DMPSV props is to check the props (with memcmp) below, which is how Mismatched is set. This sets the props to DMPSV but doesn't check that they match the props loaded from the container.

The function doesn't take PSV3 because it is checked in PSVContentVerifier::Verify (this one requires the string table indirection).

For PSV4, memcmp would be no good because of the PSV3 string index indirection, so it should probably just check the property directly, which requires adding PSVRuntimeInfo4 *PSV4 to PSVContentVerifier::VerifyEntryProperties parameters. I'll submit suggestions too.

tex3d · 2026-01-06T01:38:57Z

lib/DxilValidation/DxilContainerValidation.cpp

 void PSVContentVerifier::VerifyEntryProperties(const ShaderModel *SM,
                                               PSVRuntimeInfo0 *PSV0,
                                               PSVRuntimeInfo1 *PSV1,
                                               PSVRuntimeInfo2 *PSV2) {


I think this should have PSV3 and PSV4 pointers as well.

Suggested change

PSVRuntimeInfo2 *PSV2) {

PSVRuntimeInfo2 *PSV2,

PSVRuntimeInfo4 *PSV4) {

tex3d · 2026-01-06T01:39:04Z

lib/DxilValidation/DxilContainerValidation.cpp

  else
    Mismatched = memcmp(PSV0, &DMPSV, sizeof(PSVRuntimeInfo0)) != 0;

  if (Mismatched) {


Suggested change

if (Mismatched) {

if (PSV4 && PSV4->GroupSharedLimit != DMPSV.GroupSharedLimit)

Mismatched = true;

if (Mismatched) {

tex3d · 2026-01-06T01:57:41Z

tools/clang/lib/CodeGen/CGHLSLMS.cpp

+    // Only valid for SM6.10+
+    if (!SM->IsSM610Plus()) {
+      unsigned DiagID = Diags.getCustomDiagID(
+          DiagnosticsEngine::Error, "attribute GroupSharedLimit only valid for "


I believe we prefer to place these diagnostics earlier, in SemaHLSL.cpp (probably under DiagnoseEntry and DiagnoseEntryAttrAllowedOnStage) (in spite of the fact that many existing diagnostics were placed here in the past). Note that this allows for diagnosing library entry points too.

tex3d · 2026-01-06T02:13:31Z

tools/clang/lib/CodeGen/CGHLSLMS.cpp

+    if (SM->IsMS()) { // Fallback to default limits
+      funcProps->groupSharedLimitBytes = DXIL::kMaxMSSMSize; // 28k For MS
+    } else if (SM->IsAS() || SM->IsCS()) {


I don't think relying on SM->IsMS() will work for library targets. I think you want to use isMS (and so on) instead. That can cover the node case as well.

Suggested change

if (SM->IsMS()) { // Fallback to default limits

funcProps->groupSharedLimitBytes = DXIL::kMaxMSSMSize; // 28k For MS

} else if (SM->IsAS() || SM->IsCS()) {

if (isMS) { // Fallback to default limits

funcProps->groupSharedLimitBytes = DXIL::kMaxMSSMSize; // 28k For MS

} else if (isAS || isCS || isNode) {

tex3d · 2026-01-06T02:39:07Z

lib/DxilValidation/DxilValidation.cpp

+
+  // Check if the entry function has attribute to override TGSM size.
+  if (M.HasDxilEntryProps(M.GetEntryFunction())) {
+    DxilEntryProps &EntryProps = M.GetDxilEntryProps(M.GetEntryFunction());


Since this code only checks non-library shaders, it seems we are missing library TGSM size validation. So, the node case is completely unchecked.

However, we do calculate and store GroupSharedBytesUsed in RDAT shader info structs. This could be used for runtime checks against device capabilities instead of a maximum size.

It looks to me like we have a bit of a design gap here. Wouldn't it be better to report the maximum size used (like GroupSharedBytesUsed) to the runtime for the runtime check?

For the compilation, we can check against the max size attribute to prevent accidental over-allocation, and for validation, we can check against the size reported in PSV0 or RDAT, as well as a version-based check (SM 6.10 required larger than default max).

Then you don't need max size in PSV0 or RDAT, just reported usage. The attribute would just drive a compile-time override of default maximum.

I filed an issue on the proposal for this here: microsoft/hlsl-specs#761

Jack Elliott added 2 commits November 4, 2025 13:24

Add GroupSharedLimit as a shader attribute

795cf94

Initial implementation of GroupSharedLimit

214553d

github-project-automation bot added this to HLSL Roadmap Nov 4, 2025

github-project-automation bot moved this to New in HLSL Roadmap Nov 4, 2025

damyanp reviewed Nov 5, 2025

View reviewed changes

Jack Elliott and others added 6 commits November 6, 2025 10:03

Handle difference between MS and CS/AS and add a simple test

9b2e89d

clang format

1084671

Missed the HLSL file

87f85b4

Roll back clang format stuff

08881e5

Properly format this time

99cd065

Update naming to match spec

4da0fb8

JoeCitizen marked this pull request as ready for review December 1, 2025 19:10

damyanp linked an issue Jan 5, 2026 that may be closed by this pull request

[SM 6.10] Groupshared Memory Size preview #8038

Open

tex3d reviewed Jan 6, 2026

View reviewed changes

tex3d mentioned this pull request Jan 6, 2026

[0049] Groupshared limit vs usage microsoft/hlsl-specs#761

Open

	void hlsl::SetShaderProps(PSVRuntimeInfo4 *pInfo4, const DxilModule &DM) {
	void hlsl::SetShaderProps(PSVRuntimeInfo4 *Info4, const DxilModule &DM) {

	PSVRuntimeInfo2 *PSV2) {
	PSVRuntimeInfo2 *PSV2,
	PSVRuntimeInfo4 *PSV4) {

Implementation of GroupSharedLimit to allow increased GroupSharedMemory #7871

Are you sure you want to change the base?

Implementation of GroupSharedLimit to allow increased GroupSharedMemory #7871

Conversation

JoeCitizen commented Nov 4, 2025

Uh oh!

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

Uh oh!

Uh oh!

damyanp 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!

tex3d Jan 6, 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Nov 4, 2025 •

edited

Loading

tex3d Jan 6, 2026 •

edited

Loading