From 54501962f60b87d5ea0790302138c6db81cc2f23 Mon Sep 17 00:00:00 2001 From: jduncanator <1518948+jduncanator@users.noreply.github.com> Date: Fri, 6 Mar 2020 11:09:49 +1100 Subject: [PATCH] Fix branch with CC and predicate, and a case of SYNC propagation (#967) --- Ryujinx.Graphics.Shader/Decoders/Decoder.cs | 25 +++++++++++++++++-- .../Instructions/InstEmitFlow.cs | 20 ++++++++++++--- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/Ryujinx.Graphics.Shader/Decoders/Decoder.cs b/Ryujinx.Graphics.Shader/Decoders/Decoder.cs index 8a502e3c..cda88302 100644 --- a/Ryujinx.Graphics.Shader/Decoders/Decoder.cs +++ b/Ryujinx.Graphics.Shader/Decoders/Decoder.cs @@ -390,7 +390,14 @@ namespace Ryujinx.Graphics.Shader.Decoders void Push(PathBlockState pbs) { - if (pbs.Block == null || visited.Add(pbs.Block)) + // When block is null, this means we are pushing a restore operation. + // Restore operations are used to undo the work done inside a block + // when we return from it, for example it pops addresses pushed by + // SSY/PBK instructions inside the block, and pushes addresses poped + // by SYNC/BRK. + // For blocks, if it's already visited, we just ignore to avoid going + // around in circles and getting stuck here. + if (pbs.Block == null || !visited.Contains(pbs.Block)) { workQueue.Push(pbs); } @@ -409,6 +416,14 @@ namespace Ryujinx.Graphics.Shader.Decoders Block current = pbs.Block; + // If the block was already processed, we just ignore it, otherwise + // we would push the same child blocks of an already processed block, + // and go around in circles until memory is exhausted. + if (!visited.Add(current)) + { + continue; + } + int pushOpsCount = current.PushOpCodes.Count; if (pushOpsCount != 0) @@ -434,7 +449,9 @@ namespace Ryujinx.Graphics.Shader.Decoders } else if (current.GetLastOp() is OpCodeBranchIndir brIndir) { - foreach (Block possibleTarget in brIndir.PossibleTargets) + // By adding them in descending order (sorted by address), we process the blocks + // in order (of ascending address), since we work with a LIFO. + foreach (Block possibleTarget in brIndir.PossibleTargets.OrderByDescending(x => x.Address)) { Push(new PathBlockState(possibleTarget)); } @@ -453,6 +470,10 @@ namespace Ryujinx.Graphics.Shader.Decoders } else { + // First we push the target address (this will be used to push the + // address back into the SSY/PBK stack when we return from that block), + // then we push the block itself into the work "queue" (well, it's a stack) + // for processing. Push(new PathBlockState(targetAddress)); Push(new PathBlockState(blocks[targetAddress])); } diff --git a/Ryujinx.Graphics.Shader/Instructions/InstEmitFlow.cs b/Ryujinx.Graphics.Shader/Instructions/InstEmitFlow.cs index 0b96d751..4b058a84 100644 --- a/Ryujinx.Graphics.Shader/Instructions/InstEmitFlow.cs +++ b/Ryujinx.Graphics.Shader/Instructions/InstEmitFlow.cs @@ -145,10 +145,24 @@ namespace Ryujinx.Graphics.Shader.Instructions if (op is OpCodeBranch opBranch && opBranch.Condition != Condition.Always) { - pred = context.BitwiseAnd(pred, GetCondition(context, opBranch.Condition)); - } + Operand cond = GetCondition(context, opBranch.Condition); - if (op.Predicate.IsPT) + if (op.Predicate.IsPT) + { + pred = cond; + } + else if (op.InvertPredicate) + { + pred = context.BitwiseAnd(context.BitwiseNot(pred), cond); + } + else + { + pred = context.BitwiseAnd(pred, cond); + } + + context.BranchIfTrue(label, pred); + } + else if (op.Predicate.IsPT) { context.Branch(label); }