From 7ba3b2578bc9dfffc8e1f47ec88e4aedce66205f Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Fri, 24 Nov 2023 09:44:18 +0100 Subject: [PATCH 1/4] Add GloballyCoherentAttribute type --- .../Attributes/GloballyCoherentAttribute.cs | 26 +++++++++++++++++++ .../Core/Attributes/GroupSharedAttribute.cs | 2 +- 2 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 src/ComputeSharp/Core/Attributes/GloballyCoherentAttribute.cs diff --git a/src/ComputeSharp/Core/Attributes/GloballyCoherentAttribute.cs b/src/ComputeSharp/Core/Attributes/GloballyCoherentAttribute.cs new file mode 100644 index 000000000..a04f23d84 --- /dev/null +++ b/src/ComputeSharp/Core/Attributes/GloballyCoherentAttribute.cs @@ -0,0 +1,26 @@ +using System; + +namespace ComputeSharp; + +/// +/// An attribute that indicates that a field in +/// a shader type should use the modifier. +/// +/// +/// +/// The modifier causes memory barriers and syncs to +/// flush data across the entire GPU such that other groups can see writes. Without this +/// modifier, a memory barrier or sync will only flush a UAV within the current thread group. +/// +/// +/// In other words, if compute shader thread in a given thread group needs to perform loads of +/// data that was written by atomics or stores in another thread group, the UAV slot where the +/// data resides must be tagged as , so the implementation +/// can ignore the local cache. Otherwise, this form of cross-thread group data sharing will +/// produce undefined results. +/// +/// +[AttributeUsage(AttributeTargets.Field, AllowMultiple = false)] +public sealed class GloballyCoherentAttribute : Attribute +{ +} \ No newline at end of file diff --git a/src/ComputeSharp/Core/Attributes/GroupSharedAttribute.cs b/src/ComputeSharp/Core/Attributes/GroupSharedAttribute.cs index e53897efb..178c81fcc 100644 --- a/src/ComputeSharp/Core/Attributes/GroupSharedAttribute.cs +++ b/src/ComputeSharp/Core/Attributes/GroupSharedAttribute.cs @@ -22,7 +22,7 @@ namespace ComputeSharp; public sealed class GroupSharedAttribute : Attribute { /// - /// Creates a new instance with the specified parameters. + /// Creates a new instance. /// public GroupSharedAttribute() { From c661ef7e80d7cfa6641540830df0a18119109bdf Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Fri, 24 Nov 2023 09:53:05 +0100 Subject: [PATCH 2/4] Support [GloballyCoherent] in generator --- .../Mappings/HlslKnownKeywords.cs | 10 ++++++---- .../ComputeShaderDescriptorGenerator.HlslSource.cs | 11 +++++++++-- .../Mappings/HlslKnownTypes.cs | 10 ++++++++++ 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/ComputeSharp.SourceGeneration.Hlsl/Mappings/HlslKnownKeywords.cs b/src/ComputeSharp.SourceGeneration.Hlsl/Mappings/HlslKnownKeywords.cs index 6d706a5a7..5d4122986 100644 --- a/src/ComputeSharp.SourceGeneration.Hlsl/Mappings/HlslKnownKeywords.cs +++ b/src/ComputeSharp.SourceGeneration.Hlsl/Mappings/HlslKnownKeywords.cs @@ -4,6 +4,8 @@ using System.Reflection; using ComputeSharp.Core.Intrinsics.Attributes; +#pragma warning disable IDE0055 + namespace ComputeSharp.SourceGeneration.Mappings; /// @@ -22,8 +24,8 @@ internal static partial class HlslKnownKeywords private static IReadOnlyCollection BuildKnownKeywordsMap() { // HLSL keywords - HashSet knownKeywords = new(new[] - { + HashSet knownKeywords = new( + [ "asm", "asm_fragment", "cbuffer", "buffer", "texture", "centroid", "column_major", "compile", "discard", "dword", "export", "fxgroup", "groupshared", "half", "inline", "inout", "line", "lineadj", "linear", @@ -40,9 +42,9 @@ private static IReadOnlyCollection BuildKnownKeywordsMap() "Texture3D", "Texture2DMS", "Texture2DMSArray", "TextureCube", "TextureCubeArray", "SV_DispatchThreadID", "SV_DomainLocation", "SV_GroupID", "SV_GroupIndex", "SV_GroupThreadID", "SV_GSInstanceID", "SV_InsideTessFactor", "SV_OutputControlPointID", "SV_TessFactor", - "SV_InnerCoverage", "SV_StencilRef" + "SV_InnerCoverage", "SV_StencilRef", "globallycoherent" - }); + ]); // HLSL primitive names foreach (Type? type in HlslKnownTypes.KnownVectorTypes.Concat(HlslKnownTypes.KnownMatrixTypes)) diff --git a/src/ComputeSharp.SourceGenerators/ComputeShaderDescriptorGenerator.HlslSource.cs b/src/ComputeSharp.SourceGenerators/ComputeShaderDescriptorGenerator.HlslSource.cs index ed3798bc8..febbcfbeb 100644 --- a/src/ComputeSharp.SourceGenerators/ComputeShaderDescriptorGenerator.HlslSource.cs +++ b/src/ComputeSharp.SourceGenerators/ComputeShaderDescriptorGenerator.HlslSource.cs @@ -137,10 +137,10 @@ private static ( continue; } - AttributeData? attribute = fieldSymbol.GetAttributes().FirstOrDefault(static a => a.AttributeClass?.ToDisplayString() == "ComputeSharp.GroupSharedAttribute"); + AttributeData? groupSharedAttribute = fieldSymbol.GetAttributes().FirstOrDefault(static a => a.AttributeClass?.ToDisplayString() == "ComputeSharp.GroupSharedAttribute"); // Group shared fields must be static - if (attribute is not null) + if (groupSharedAttribute is not null) { diagnostics.Add(InvalidGroupSharedFieldDeclaration, fieldSymbol, structDeclarationSymbol, fieldName); } @@ -169,6 +169,13 @@ private static ( types.Add((INamedTypeSymbol)typeSymbol.TypeArguments[0]); } + // Check whether the resource is a globallycoherent writeable buffer + if (HlslKnownTypes.IsReadWriteBufferType(metadataName) && + fieldSymbol.TryGetAttributeWithFullyQualifiedMetadataName("ComputeSharp.GloballyCoherentAttribute", out _)) + { + typeName = "globallycoherent " + typeName; + } + // Add the current mapping for the name (if the name used a reserved keyword) resources.Add((metadataName, mapping ?? fieldName, typeName)); } diff --git a/src/ComputeSharp.SourceGenerators/Mappings/HlslKnownTypes.cs b/src/ComputeSharp.SourceGenerators/Mappings/HlslKnownTypes.cs index 887e985b2..3f9b88796 100644 --- a/src/ComputeSharp.SourceGenerators/Mappings/HlslKnownTypes.cs +++ b/src/ComputeSharp.SourceGenerators/Mappings/HlslKnownTypes.cs @@ -29,6 +29,16 @@ public static bool IsConstantBufferType(string typeName) return typeName == "ComputeSharp.ConstantBuffer`1"; } + /// + /// Checks whether or not a given type name matches a read write buffer type. + /// + /// The input type name to check. + /// Whether or not represents a read write buffer type. + public static bool IsReadWriteBufferType(string typeName) + { + return typeName == "ComputeSharp.ReadWriteBuffer`1"; + } + /// /// Checks whether or not a given type name matches a structured buffer type. /// From 2f0d7911d66eee748c476aa8af3d0398a8a487d0 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Fri, 24 Nov 2023 09:53:21 +0100 Subject: [PATCH 3/4] Add unit test for [GloballyCoherent] buffer --- .../ComputeSharp.Tests/ShaderCompilerTests.cs | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/tests/ComputeSharp.Tests/ShaderCompilerTests.cs b/tests/ComputeSharp.Tests/ShaderCompilerTests.cs index afdbb2562..71e22aeb0 100644 --- a/tests/ComputeSharp.Tests/ShaderCompilerTests.cs +++ b/tests/ComputeSharp.Tests/ShaderCompilerTests.cs @@ -607,6 +607,58 @@ public void Execute() this.buffer[ThreadIds.X] = ThreadIds.X; } } + + [TestMethod] + public void GloballyCoherentBuffers() + { + ShaderInfo shaderInfo = ReflectionServices.GetShaderInfo(); + + Assert.AreEqual( + """ + // ================================================ + // AUTO GENERATED + // ================================================ + // This shader was created by ComputeSharp. + // See: https://github.com/Sergio0694/ComputeSharp. + + #define __GroupSize__get_X 64 + #define __GroupSize__get_Y 1 + #define __GroupSize__get_Z 1 + + cbuffer _ : register(b0) + { + uint __x; + uint __y; + uint __z; + } + + globallycoherent RWStructuredBuffer __reserved__buffer : register(u0); + + [NumThreads(__GroupSize__get_X, __GroupSize__get_Y, __GroupSize__get_Z)] + void Execute(uint3 ThreadIds : SV_DispatchThreadID) + { + if (ThreadIds.x < __x && ThreadIds.y < __y && ThreadIds.z < __z) + { + InterlockedAdd(__reserved__buffer[0], 1); + } + } + """, + shaderInfo.HlslSource); + } + + [AutoConstructor] + [ThreadGroupSize(DefaultThreadGroupSizes.X)] + [GeneratedComputeShaderDescriptor] + internal readonly partial struct GloballyCoherentBufferShader : IComputeShader + { + [GloballyCoherent] + private readonly ReadWriteBuffer buffer; + + public void Execute() + { + Hlsl.InterlockedAdd(ref this.buffer[0], 1); + } + } } } From e1ca46aa4cdeee72e292234ddf365e724c895fbe Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Fri, 24 Nov 2023 10:12:25 +0100 Subject: [PATCH 4/4] Add InvalidGloballyCoherentFieldDeclarationAnalyzer --- .../AnalyzerReleases.Shipped.md | 1 + ...loballyCoherentFieldDeclarationAnalyzer.cs | 62 ++++++++++++++ .../Diagnostics/DiagnosticDescriptors.cs | 16 ++++ .../Test_Analyzers.cs | 84 +++++++++++++++++++ .../Test_D2DPixelShaderSourceGenerator.cs | 1 + 5 files changed, 164 insertions(+) create mode 100644 src/ComputeSharp.SourceGenerators/Diagnostics/Analyzers/InvalidGloballyCoherentFieldDeclarationAnalyzer.cs diff --git a/src/ComputeSharp.SourceGenerators/AnalyzerReleases.Shipped.md b/src/ComputeSharp.SourceGenerators/AnalyzerReleases.Shipped.md index 2be10a4e5..4157950ae 100644 --- a/src/ComputeSharp.SourceGenerators/AnalyzerReleases.Shipped.md +++ b/src/ComputeSharp.SourceGenerators/AnalyzerReleases.Shipped.md @@ -63,3 +63,4 @@ CMPS0054 | ComputeSharp.Shaders | Error | [Documentation](https://github.com/Ser CMPS0055 | ComputeSharp.Shaders | Error | [Documentation](https://github.com/Sergio0694/ComputeSharp) CMPS0056 | ComputeSharp.Shaders | Error | [Documentation](https://github.com/Sergio0694/ComputeSharp) CMPS0057 | ComputeSharp.Shaders | Warning | [Documentation](https://github.com/Sergio0694/ComputeSharp) +CMPS0058 | ComputeSharp.Shaders | Error | [Documentation](https://github.com/Sergio0694/ComputeSharp) diff --git a/src/ComputeSharp.SourceGenerators/Diagnostics/Analyzers/InvalidGloballyCoherentFieldDeclarationAnalyzer.cs b/src/ComputeSharp.SourceGenerators/Diagnostics/Analyzers/InvalidGloballyCoherentFieldDeclarationAnalyzer.cs new file mode 100644 index 000000000..f3b00ae8d --- /dev/null +++ b/src/ComputeSharp.SourceGenerators/Diagnostics/Analyzers/InvalidGloballyCoherentFieldDeclarationAnalyzer.cs @@ -0,0 +1,62 @@ +using System.Collections.Immutable; +using ComputeSharp.SourceGeneration.Extensions; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using static ComputeSharp.SourceGeneration.Diagnostics.DiagnosticDescriptors; + +namespace ComputeSharp.SourceGenerators; + +/// +/// A diagnostic analyzer that generates an error whenever the [GloballyCoherent] attribute is used on an invalid target field. +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class InvalidGloballyCoherentFieldDeclarationAnalyzer : DiagnosticAnalyzer +{ + /// + public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create(InvalidGloballyCoherentFieldDeclaration); + + /// + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics); + context.EnableConcurrentExecution(); + + context.RegisterCompilationStartAction(static context => + { + // Get the IComputeShader, IComputeShader, ReadWriteBuffer and [GloballyCoherent] symbols + if (context.Compilation.GetTypeByMetadataName("ComputeSharp.IComputeShader") is not { } computeShaderSymbol || + context.Compilation.GetTypeByMetadataName("ComputeSharp.IComputeShader`1") is not { } pixelShaderSymbol || + context.Compilation.GetTypeByMetadataName("ComputeSharp.ReadWriteBuffer`1") is not { } readWriteBufferSymbol || + context.Compilation.GetTypeByMetadataName("ComputeSharp.GloballyCoherentAttribute") is not { } globallyCoherentAttributeSymbol) + { + return; + } + + context.RegisterSymbolAction(context => + { + if (context.Symbol is not IFieldSymbol { ContainingType: { } typeSymbol } fieldSymbol) + { + return; + } + + // If the current type does not have [GloballyCoherent], there is nothing to do + if (!fieldSymbol.TryGetAttributeWithType(globallyCoherentAttributeSymbol, out AttributeData? attribute)) + { + return; + } + + // Emit a diagnostic if the field is not valid (either static, not of ReadWriteBuffer type, or not within a compute shader type) + if (fieldSymbol.IsStatic || + fieldSymbol.Type is not INamedTypeSymbol { IsGenericType: true } fieldTypeSymbol || + !SymbolEqualityComparer.Default.Equals(fieldTypeSymbol.ConstructedFrom, readWriteBufferSymbol) || + !MissingComputeShaderDescriptorOnComputeShaderAnalyzer.IsComputeShaderType(typeSymbol, computeShaderSymbol, pixelShaderSymbol)) + { + context.ReportDiagnostic(Diagnostic.Create( + InvalidGloballyCoherentFieldDeclaration, + attribute.GetLocation(), + fieldSymbol)); + } + }, SymbolKind.Field); + }); + } +} \ No newline at end of file diff --git a/src/ComputeSharp.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs b/src/ComputeSharp.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs index 1219c4d19..a879aabb2 100644 --- a/src/ComputeSharp.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs +++ b/src/ComputeSharp.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs @@ -814,4 +814,20 @@ partial class DiagnosticDescriptors isEnabledByDefault: true, description: "Shader types should be readonly (shaders cannot mutate their instance state while running, so shader types not being readonly makes them error prone).", helpLinkUri: "https://github.com/Sergio0694/ComputeSharp"); + + /// + /// Gets a for when a field annotated with [GloballyCoherent] is not valid. + /// + /// Format: "The field "{0}" is annotated with [GloballyCoherent], but is not a valid target for it (only ReadWriteBuffer<T> instance fields in compute shader types can be annotated with [GloballyCoherent])". + /// + /// + public static readonly DiagnosticDescriptor InvalidGloballyCoherentFieldDeclaration = new( + id: "CMPS0058", + title: "Invalid [GloballyCoherent] field declaration", + messageFormat: """The field "{0}" is annotated with [GloballyCoherent], but is not a valid target for it (only ReadWriteBuffer instance fields in compute shader types can be annotated with [GloballyCoherent])""", + category: "ComputeSharp.Shaders", + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true, + description: "The [GloballyCoherent] attribute is only valid on ReadWriteBuffer instance fields in compute shader types.", + helpLinkUri: "https://github.com/Sergio0694/ComputeSharp"); } \ No newline at end of file diff --git a/tests/ComputeSharp.Tests.SourceGenerators/Test_Analyzers.cs b/tests/ComputeSharp.Tests.SourceGenerators/Test_Analyzers.cs index 113cd6574..a489605f9 100644 --- a/tests/ComputeSharp.Tests.SourceGenerators/Test_Analyzers.cs +++ b/tests/ComputeSharp.Tests.SourceGenerators/Test_Analyzers.cs @@ -85,4 +85,88 @@ public void Execute() await CSharpAnalyzerWithLanguageVersionTest.VerifyAnalyzerAsync(source); } + + [TestMethod] + public async Task GloballyCoherentAttribute_NotWithinShaderType() + { + const string source = """ + using ComputeSharp; + + namespace MyFancyApp.Sample; + + internal partial struct MyType + { + [{|CMPS0058:GloballyCoherent|}] + public ReadWriteBuffer buffer; + } + """; + + await CSharpAnalyzerWithLanguageVersionTest.VerifyAnalyzerAsync(source); + } + + [TestMethod] + public async Task GloballyCoherentAttribute_IncorrectType() + { + const string source = """ + using ComputeSharp; + + namespace MyFancyApp.Sample; + + internal partial struct MyType : IComputeShader + { + [{|CMPS0058:GloballyCoherent|}] + public ReadOnlyBuffer buffer; + + public void Execute() + { + } + } + """; + + await CSharpAnalyzerWithLanguageVersionTest.VerifyAnalyzerAsync(source); + } + + [TestMethod] + public async Task GloballyCoherentAttribute_StaticField() + { + const string source = """ + using ComputeSharp; + + namespace MyFancyApp.Sample; + + internal partial struct MyType : IComputeShader + { + [{|CMPS0058:GloballyCoherent|}] + public static ReadWriteBuffer buffer; + + public void Execute() + { + } + } + """; + + await CSharpAnalyzerWithLanguageVersionTest.VerifyAnalyzerAsync(source); + } + + [TestMethod] + public async Task GloballyCoherentAttribute_ValidField_DoesNotWarn() + { + const string source = """ + using ComputeSharp; + + namespace MyFancyApp.Sample; + + internal partial struct MyType : IComputeShader + { + [GloballyCoherent] + public ReadWriteBuffer buffer; + + public void Execute() + { + } + } + """; + + await CSharpAnalyzerWithLanguageVersionTest.VerifyAnalyzerAsync(source); + } } \ No newline at end of file diff --git a/tests/ComputeSharp.Tests.SourceGenerators/Test_D2DPixelShaderSourceGenerator.cs b/tests/ComputeSharp.Tests.SourceGenerators/Test_D2DPixelShaderSourceGenerator.cs index 86ed4d84b..d62829e7b 100644 --- a/tests/ComputeSharp.Tests.SourceGenerators/Test_D2DPixelShaderSourceGenerator.cs +++ b/tests/ComputeSharp.Tests.SourceGenerators/Test_D2DPixelShaderSourceGenerator.cs @@ -895,6 +895,7 @@ private static async Task VerifyGeneratedDiagnosticsAsync(string source, (string // Also validate all analyzers await CSharpAnalyzerWithLanguageVersionTest.VerifyAnalyzerAsync(source); await CSharpAnalyzerWithLanguageVersionTest.VerifyAnalyzerAsync(source); + await CSharpAnalyzerWithLanguageVersionTest.VerifyAnalyzerAsync(source); await CSharpAnalyzerWithLanguageVersionTest.VerifyAnalyzerAsync(source); await CSharpAnalyzerWithLanguageVersionTest.VerifyAnalyzerAsync(source); await CSharpAnalyzerWithLanguageVersionTest.VerifyAnalyzerAsync(source);