From 3b375525fbaded422fb2f9a9984a5770a3779fcb Mon Sep 17 00:00:00 2001 From: gdkchan Date: Fri, 26 May 2023 15:19:37 -0300 Subject: [PATCH] Force reciprocal operation with value biased by constant to be precise on macOS (#5110) * Force operations to be precise in some cases on SPIR-V * Make it a bit more strict, add comments * Shader cache version bump --- .../Shader/DiskCache/DiskCacheHostStorage.cs | 2 +- .../CodeGen/Spirv/Instructions.cs | 8 +++--- .../IntermediateRepresentation/Operation.cs | 2 ++ .../StructuredIr/AstOperation.cs | 14 +++++++--- .../StructuredIr/AstTextureOperation.cs | 2 +- .../StructuredIr/StructuredProgram.cs | 26 ++++++++++++++++--- .../StructuredIr/StructuredProgramContext.cs | 2 +- .../Translation/Rewriter.cs | 25 ++++++++++++++++++ 8 files changed, 67 insertions(+), 14 deletions(-) diff --git a/src/Ryujinx.Graphics.Gpu/Shader/DiskCache/DiskCacheHostStorage.cs b/src/Ryujinx.Graphics.Gpu/Shader/DiskCache/DiskCacheHostStorage.cs index f4d7f3b58c..e2346bd0b8 100644 --- a/src/Ryujinx.Graphics.Gpu/Shader/DiskCache/DiskCacheHostStorage.cs +++ b/src/Ryujinx.Graphics.Gpu/Shader/DiskCache/DiskCacheHostStorage.cs @@ -22,7 +22,7 @@ namespace Ryujinx.Graphics.Gpu.Shader.DiskCache private const ushort FileFormatVersionMajor = 1; private const ushort FileFormatVersionMinor = 2; private const uint FileFormatVersionPacked = ((uint)FileFormatVersionMajor << 16) | FileFormatVersionMinor; - private const uint CodeGenVersion = 5102; + private const uint CodeGenVersion = 5110; private const string SharedTocFileName = "shared.toc"; private const string SharedDataFileName = "shared.data"; diff --git a/src/Ryujinx.Graphics.Shader/CodeGen/Spirv/Instructions.cs b/src/Ryujinx.Graphics.Shader/CodeGen/Spirv/Instructions.cs index eb64f82418..f088a47f35 100644 --- a/src/Ryujinx.Graphics.Shader/CodeGen/Spirv/Instructions.cs +++ b/src/Ryujinx.Graphics.Shader/CodeGen/Spirv/Instructions.cs @@ -2245,7 +2245,7 @@ namespace Ryujinx.Graphics.Shader.CodeGen.Spirv { var result = emitF(context.TypeFP64(), context.GetFP64(src1), context.GetFP64(src2)); - if (!context.Config.GpuAccessor.QueryHostReducedPrecision()) + if (!context.Config.GpuAccessor.QueryHostReducedPrecision() || operation.ForcePrecise) { context.Decorate(result, Decoration.NoContraction); } @@ -2256,7 +2256,7 @@ namespace Ryujinx.Graphics.Shader.CodeGen.Spirv { var result = emitF(context.TypeFP32(), context.GetFP32(src1), context.GetFP32(src2)); - if (!context.Config.GpuAccessor.QueryHostReducedPrecision()) + if (!context.Config.GpuAccessor.QueryHostReducedPrecision() || operation.ForcePrecise) { context.Decorate(result, Decoration.NoContraction); } @@ -2316,7 +2316,7 @@ namespace Ryujinx.Graphics.Shader.CodeGen.Spirv { var result = emitF(context.TypeFP64(), context.GetFP64(src1), context.GetFP64(src2), context.GetFP64(src3)); - if (!context.Config.GpuAccessor.QueryHostReducedPrecision()) + if (!context.Config.GpuAccessor.QueryHostReducedPrecision() || operation.ForcePrecise) { context.Decorate(result, Decoration.NoContraction); } @@ -2327,7 +2327,7 @@ namespace Ryujinx.Graphics.Shader.CodeGen.Spirv { var result = emitF(context.TypeFP32(), context.GetFP32(src1), context.GetFP32(src2), context.GetFP32(src3)); - if (!context.Config.GpuAccessor.QueryHostReducedPrecision()) + if (!context.Config.GpuAccessor.QueryHostReducedPrecision() || operation.ForcePrecise) { context.Decorate(result, Decoration.NoContraction); } diff --git a/src/Ryujinx.Graphics.Shader/IntermediateRepresentation/Operation.cs b/src/Ryujinx.Graphics.Shader/IntermediateRepresentation/Operation.cs index 99179f1519..d502a9b659 100644 --- a/src/Ryujinx.Graphics.Shader/IntermediateRepresentation/Operation.cs +++ b/src/Ryujinx.Graphics.Shader/IntermediateRepresentation/Operation.cs @@ -8,6 +8,8 @@ namespace Ryujinx.Graphics.Shader.IntermediateRepresentation public Instruction Inst { get; private set; } public StorageKind StorageKind { get; } + public bool ForcePrecise { get; set; } + private Operand[] _dests; public Operand Dest diff --git a/src/Ryujinx.Graphics.Shader/StructuredIr/AstOperation.cs b/src/Ryujinx.Graphics.Shader/StructuredIr/AstOperation.cs index 2393fd8d83..4cf729d096 100644 --- a/src/Ryujinx.Graphics.Shader/StructuredIr/AstOperation.cs +++ b/src/Ryujinx.Graphics.Shader/StructuredIr/AstOperation.cs @@ -10,6 +10,7 @@ namespace Ryujinx.Graphics.Shader.StructuredIr { public Instruction Inst { get; } public StorageKind StorageKind { get; } + public bool ForcePrecise { get; } public int Index { get; } @@ -17,10 +18,11 @@ namespace Ryujinx.Graphics.Shader.StructuredIr public int SourcesCount => _sources.Length; - public AstOperation(Instruction inst, StorageKind storageKind, IAstNode[] sources, int sourcesCount) + public AstOperation(Instruction inst, StorageKind storageKind, bool forcePrecise, IAstNode[] sources, int sourcesCount) { Inst = inst; StorageKind = storageKind; + ForcePrecise = forcePrecise; _sources = sources; for (int index = 0; index < sources.Length; index++) @@ -38,12 +40,18 @@ namespace Ryujinx.Graphics.Shader.StructuredIr Index = 0; } - public AstOperation(Instruction inst, StorageKind storageKind, int index, IAstNode[] sources, int sourcesCount) : this(inst, storageKind, sources, sourcesCount) + public AstOperation( + Instruction inst, + StorageKind storageKind, + bool forcePrecise, + int index, + IAstNode[] sources, + int sourcesCount) : this(inst, storageKind, forcePrecise, sources, sourcesCount) { Index = index; } - public AstOperation(Instruction inst, params IAstNode[] sources) : this(inst, StorageKind.None, sources, sources.Length) + public AstOperation(Instruction inst, params IAstNode[] sources) : this(inst, StorageKind.None, false, sources, sources.Length) { } diff --git a/src/Ryujinx.Graphics.Shader/StructuredIr/AstTextureOperation.cs b/src/Ryujinx.Graphics.Shader/StructuredIr/AstTextureOperation.cs index 6c27279c94..a4e097eb6f 100644 --- a/src/Ryujinx.Graphics.Shader/StructuredIr/AstTextureOperation.cs +++ b/src/Ryujinx.Graphics.Shader/StructuredIr/AstTextureOperation.cs @@ -19,7 +19,7 @@ namespace Ryujinx.Graphics.Shader.StructuredIr int cbufSlot, int handle, int index, - params IAstNode[] sources) : base(inst, StorageKind.None, index, sources, sources.Length) + params IAstNode[] sources) : base(inst, StorageKind.None, false, index, sources, sources.Length) { Type = type; Format = format; diff --git a/src/Ryujinx.Graphics.Shader/StructuredIr/StructuredProgram.cs b/src/Ryujinx.Graphics.Shader/StructuredIr/StructuredProgram.cs index 6846245e61..4405c07aae 100644 --- a/src/Ryujinx.Graphics.Shader/StructuredIr/StructuredProgram.cs +++ b/src/Ryujinx.Graphics.Shader/StructuredIr/StructuredProgram.cs @@ -156,7 +156,13 @@ namespace Ryujinx.Graphics.Shader.StructuredIr } else { - source = new AstOperation(inst, operation.StorageKind, operation.Index, sources, operation.SourcesCount); + source = new AstOperation( + inst, + operation.StorageKind, + operation.ForcePrecise, + operation.Index, + sources, + operation.SourcesCount); } AggregateType destElemType = destType; @@ -179,7 +185,7 @@ namespace Ryujinx.Graphics.Shader.StructuredIr dest.VarType = destElemType; - context.AddNode(new AstAssignment(dest, new AstOperation(Instruction.VectorExtract, StorageKind.None, new[] { destVec, index }, 2))); + context.AddNode(new AstAssignment(dest, new AstOperation(Instruction.VectorExtract, StorageKind.None, false, new[] { destVec, index }, 2))); } } else if (operation.Dest != null) @@ -227,7 +233,13 @@ namespace Ryujinx.Graphics.Shader.StructuredIr } else if (!isCopy) { - source = new AstOperation(inst, operation.StorageKind, operation.Index, sources, operation.SourcesCount); + source = new AstOperation( + inst, + operation.StorageKind, + operation.ForcePrecise, + operation.Index, + sources, + operation.SourcesCount); } else { @@ -248,7 +260,13 @@ namespace Ryujinx.Graphics.Shader.StructuredIr } else { - context.AddNode(new AstOperation(inst, operation.StorageKind, operation.Index, sources, operation.SourcesCount)); + context.AddNode(new AstOperation( + inst, + operation.StorageKind, + operation.ForcePrecise, + operation.Index, + sources, + operation.SourcesCount)); } // Those instructions needs to be emulated by using helper functions, diff --git a/src/Ryujinx.Graphics.Shader/StructuredIr/StructuredProgramContext.cs b/src/Ryujinx.Graphics.Shader/StructuredIr/StructuredProgramContext.cs index c5ad3683a4..a4d0799148 100644 --- a/src/Ryujinx.Graphics.Shader/StructuredIr/StructuredProgramContext.cs +++ b/src/Ryujinx.Graphics.Shader/StructuredIr/StructuredProgramContext.cs @@ -319,7 +319,7 @@ namespace Ryujinx.Graphics.Shader.StructuredIr new AstOperand(OperandType.Constant, elemIndex) }; - return new AstOperation(Instruction.Load, StorageKind.ConstantBuffer, sources, sources.Length); + return new AstOperation(Instruction.Load, StorageKind.ConstantBuffer, false, sources, sources.Length); } return GetOperand(operand); diff --git a/src/Ryujinx.Graphics.Shader/Translation/Rewriter.cs b/src/Ryujinx.Graphics.Shader/Translation/Rewriter.cs index eab0f9d2f0..866ae5223b 100644 --- a/src/Ryujinx.Graphics.Shader/Translation/Rewriter.cs +++ b/src/Ryujinx.Graphics.Shader/Translation/Rewriter.cs @@ -14,6 +14,7 @@ namespace Ryujinx.Graphics.Shader.Translation public static void RunPass(HelperFunctionManager hfm, BasicBlock[] blocks, ShaderConfig config) { bool isVertexShader = config.Stage == ShaderStage.Vertex; + bool isImpreciseFragmentShader = config.Stage == ShaderStage.Fragment && config.GpuAccessor.QueryHostReducedPrecision(); bool hasConstantBufferDrawParameters = config.GpuAccessor.QueryHasConstantBufferDrawParameters(); bool hasVectorIndexingBug = config.GpuAccessor.QueryHostHasVectorIndexingBug(); bool supportsSnormBufferTextureFormat = config.GpuAccessor.QueryHostSupportsSnormBufferTextureFormat(); @@ -45,6 +46,11 @@ namespace Ryujinx.Graphics.Shader.Translation } } + if (isImpreciseFragmentShader) + { + EnableForcePreciseIfNeeded(operation); + } + if (hasVectorIndexingBug) { InsertVectorComponentSelect(node, config); @@ -81,6 +87,25 @@ namespace Ryujinx.Graphics.Shader.Translation } } + private static void EnableForcePreciseIfNeeded(Operation operation) + { + // There are some cases where a small bias is added to values to prevent division by zero. + // When operating with reduced precision, it is possible for this bias to get rounded to 0 + // and cause a division by zero. + // To prevent that, we force those operations to be precise even if the host wants + // imprecise operations for performance. + + if (operation.Inst == (Instruction.FP32 | Instruction.Divide) && + operation.GetSource(0).Type == OperandType.Constant && + operation.GetSource(0).AsFloat() == 1f && + operation.GetSource(1).AsgOp is Operation addOp && + addOp.Inst == (Instruction.FP32 | Instruction.Add) && + addOp.GetSource(1).Type == OperandType.Constant) + { + addOp.ForcePrecise = true; + } + } + private static void InsertVectorComponentSelect(LinkedListNode node, ShaderConfig config) { Operation operation = (Operation)node.Value;