From 2df16ded9ba06a874b58132cc2c78175631a3b8d Mon Sep 17 00:00:00 2001 From: gdkchan Date: Sat, 15 Oct 2022 20:20:16 -0300 Subject: [PATCH] Improve shader BRX instruction code generation (#3759) * Improve shader BRX instruction code generation * Shader cache version bump, add some comments and asserts --- .../Shader/DiskCache/DiskCacheHostStorage.cs | 2 +- Ryujinx.Graphics.Shader/Decoders/Decoder.cs | 13 ++- .../Instructions/InstEmitFlowControl.cs | 82 ++++++++++++++++--- .../Translation/EmitterContext.cs | 65 +++++++++++++-- .../Translation/Translator.cs | 2 +- 5 files changed, 140 insertions(+), 24 deletions(-) diff --git a/Ryujinx.Graphics.Gpu/Shader/DiskCache/DiskCacheHostStorage.cs b/Ryujinx.Graphics.Gpu/Shader/DiskCache/DiskCacheHostStorage.cs index edb05cc074..2168c18591 100644 --- a/Ryujinx.Graphics.Gpu/Shader/DiskCache/DiskCacheHostStorage.cs +++ b/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 = 3732; + private const uint CodeGenVersion = 3759; private const string SharedTocFileName = "shared.toc"; private const string SharedDataFileName = "shared.data"; diff --git a/Ryujinx.Graphics.Shader/Decoders/Decoder.cs b/Ryujinx.Graphics.Shader/Decoders/Decoder.cs index 1c329b593b..9dafb089f7 100644 --- a/Ryujinx.Graphics.Shader/Decoders/Decoder.cs +++ b/Ryujinx.Graphics.Shader/Decoders/Decoder.cs @@ -377,6 +377,8 @@ namespace Ryujinx.Graphics.Shader.Decoders if (lastOp.Name == InstName.Brx && block.Successors.Count == (hasNext ? 1 : 0)) { + HashSet visited = new HashSet(); + InstBrx opBrx = new InstBrx(lastOp.RawOpCode); ulong baseOffset = lastOp.GetAbsoluteAddress(); @@ -392,9 +394,14 @@ namespace Ryujinx.Graphics.Shader.Decoders for (int i = 0; i < cbOffsetsCount; i++) { uint targetOffset = config.ConstantBuffer1Read(cbBaseOffset + i * 4); - Block target = getBlock(baseOffset + targetOffset); - target.Predecessors.Add(block); - block.Successors.Add(target); + ulong targetAddress = baseOffset + targetOffset; + + if (visited.Add(targetAddress)) + { + Block target = getBlock(targetAddress); + target.Predecessors.Add(block); + block.Successors.Add(target); + } } } } diff --git a/Ryujinx.Graphics.Shader/Instructions/InstEmitFlowControl.cs b/Ryujinx.Graphics.Shader/Instructions/InstEmitFlowControl.cs index aee52c517c..f1dd279c87 100644 --- a/Ryujinx.Graphics.Shader/Instructions/InstEmitFlowControl.cs +++ b/Ryujinx.Graphics.Shader/Instructions/InstEmitFlowControl.cs @@ -41,24 +41,82 @@ namespace Ryujinx.Graphics.Shader.Instructions Operand address = context.IAdd(Register(op.SrcA, RegisterType.Gpr), Const(offset)); - // Sorting the target addresses in descending order improves the code, - // since it will always check the most distant targets first, then the - // near ones. This can be easily transformed into if/else statements. - var sortedTargets = context.CurrBlock.Successors.Skip(startIndex).OrderByDescending(x => x.Address); + var targets = context.CurrBlock.Successors.Skip(startIndex); - Block lastTarget = sortedTargets.LastOrDefault(); + bool allTargetsSinglePred = true; + int total = context.CurrBlock.Successors.Count - startIndex; + int count = 0; - foreach (Block possibleTarget in sortedTargets) + foreach (var target in targets.OrderBy(x => x.Address)) { - Operand label = context.GetLabel(possibleTarget.Address); - - if (possibleTarget != lastTarget) + if (++count < total && (target.Predecessors.Count > 1 || target.Address <= context.CurrBlock.Address)) { - context.BranchIfTrue(label, context.ICompareEqual(address, Const((int)possibleTarget.Address))); + allTargetsSinglePred = false; + break; } - else + } + + if (allTargetsSinglePred) + { + // Chain blocks, each target block will check if the BRX target address + // matches its own address, if not, it jumps to the next target which will do the same check, + // until it reaches the last possible target, which executed unconditionally. + // We can only do this if the BRX block is the only predecessor of all target blocks. + // Additionally, this is not supported for blocks located before the current block, + // since it will be too late to insert a label, but this is something that can be improved + // in the future if necessary. + + var sortedTargets = targets.OrderBy(x => x.Address); + + Block currentTarget = null; + ulong firstTargetAddress = 0; + + foreach (Block nextTarget in sortedTargets) { - context.Branch(label); + if (currentTarget != null) + { + if (currentTarget.Address != nextTarget.Address) + { + context.SetBrxTarget(currentTarget.Address, address, (int)currentTarget.Address, nextTarget.Address); + } + } + else + { + firstTargetAddress = nextTarget.Address; + } + + currentTarget = nextTarget; + } + + context.Branch(context.GetLabel(firstTargetAddress)); + } + else + { + // Emit the branches sequentially. + // This generates slightly worse code, but should work for all cases. + + var sortedTargets = targets.OrderByDescending(x => x.Address); + ulong lastTargetAddress = ulong.MaxValue; + + count = 0; + + foreach (Block target in sortedTargets) + { + Operand label = context.GetLabel(target.Address); + + if (++count < total) + { + if (target.Address != lastTargetAddress) + { + context.BranchIfTrue(label, context.ICompareEqual(address, Const((int)target.Address))); + } + + lastTargetAddress = target.Address; + } + else + { + context.Branch(label); + } } } } diff --git a/Ryujinx.Graphics.Shader/Translation/EmitterContext.cs b/Ryujinx.Graphics.Shader/Translation/EmitterContext.cs index 3e50ce2fdf..ef5d7b96dd 100644 --- a/Ryujinx.Graphics.Shader/Translation/EmitterContext.cs +++ b/Ryujinx.Graphics.Shader/Translation/EmitterContext.cs @@ -21,8 +21,33 @@ namespace Ryujinx.Graphics.Shader.Translation public int OperationsCount => _operations.Count; + private struct BrxTarget + { + public readonly Operand Selector; + public readonly int ExpectedValue; + public readonly ulong NextTargetAddress; + + public BrxTarget(Operand selector, int expectedValue, ulong nextTargetAddress) + { + Selector = selector; + ExpectedValue = expectedValue; + NextTargetAddress = nextTargetAddress; + } + } + + private class BlockLabel + { + public readonly Operand Label; + public BrxTarget BrxTarget; + + public BlockLabel(Operand label) + { + Label = label; + } + } + private readonly List _operations; - private readonly Dictionary _labels; + private readonly Dictionary _labels; public EmitterContext(DecodedProgram program, ShaderConfig config, bool isNonMain) { @@ -30,7 +55,7 @@ namespace Ryujinx.Graphics.Shader.Translation Config = config; IsNonMain = isNonMain; _operations = new List(); - _labels = new Dictionary(); + _labels = new Dictionary(); EmitStart(); } @@ -158,14 +183,40 @@ namespace Ryujinx.Graphics.Shader.Translation public Operand GetLabel(ulong address) { - if (!_labels.TryGetValue(address, out Operand label)) - { - label = Label(); + return EnsureBlockLabel(address).Label; + } - _labels.Add(address, label); + public void SetBrxTarget(ulong address, Operand selector, int targetValue, ulong nextTargetAddress) + { + BlockLabel blockLabel = EnsureBlockLabel(address); + Debug.Assert(blockLabel.BrxTarget.Selector == null); + blockLabel.BrxTarget = new BrxTarget(selector, targetValue, nextTargetAddress); + } + + public void EnterBlock(ulong address) + { + BlockLabel blockLabel = EnsureBlockLabel(address); + + MarkLabel(blockLabel.Label); + + BrxTarget brxTarget = blockLabel.BrxTarget; + + if (brxTarget.Selector != null) + { + this.BranchIfFalse(GetLabel(brxTarget.NextTargetAddress), this.ICompareEqual(brxTarget.Selector, Const(brxTarget.ExpectedValue))); + } + } + + private BlockLabel EnsureBlockLabel(ulong address) + { + if (!_labels.TryGetValue(address, out BlockLabel blockLabel)) + { + blockLabel = new BlockLabel(Label()); + + _labels.Add(address, blockLabel); } - return label; + return blockLabel; } public void PrepareForVertexReturn() diff --git a/Ryujinx.Graphics.Shader/Translation/Translator.cs b/Ryujinx.Graphics.Shader/Translation/Translator.cs index 78fd9498ae..ff0de1bd0b 100644 --- a/Ryujinx.Graphics.Shader/Translation/Translator.cs +++ b/Ryujinx.Graphics.Shader/Translation/Translator.cs @@ -162,7 +162,7 @@ namespace Ryujinx.Graphics.Shader.Translation { context.CurrBlock = block; - context.MarkLabel(context.GetLabel(block.Address)); + context.EnterBlock(block.Address); EmitOps(context, block); }