From ff9c069d4431010d7f9dfa8ab2cc19f0b3660c72 Mon Sep 17 00:00:00 2001 From: kuba Date: Sun, 27 Sep 2026 01:09:42 +0200 Subject: [PATCH 1/2] [SpirvTranslator] Give each used register its own private variable The translator kept the scalar and vector register files as two private arrays (128 and 512 uints) in every module. MoltenVK lowers a private array that size to per-thread memory, so every register read and write in every invocation went through memory. Declare one private variable per register the program touches, on first use; the SPIR-V is otherwise unchanged. --- .../Gen5SpirvTranslator.cs | 92 ++++++++++++------- .../Gen5ImageTests.cs | 29 ++++-- 2 files changed, 81 insertions(+), 40 deletions(-) diff --git a/src/SharpEmu.ShaderCompiler.Vulkan/Gen5SpirvTranslator.cs b/src/SharpEmu.ShaderCompiler.Vulkan/Gen5SpirvTranslator.cs index 4c4a7b9..a9082ab 100644 --- a/src/SharpEmu.ShaderCompiler.Vulkan/Gen5SpirvTranslator.cs +++ b/src/SharpEmu.ShaderCompiler.Vulkan/Gen5SpirvTranslator.cs @@ -144,7 +144,12 @@ public static partial class Gen5SpirvTranslator private uint _privateVec2Pointer; private uint _privateBoolPointer; private uint _runtimeBufferBiases; - private uint _scalarRegisters; + // One Private variable per register the program touches, declared on first use. A whole + // register-file array is kept only for the VGPRs of a program that indexes them at run time + // (V_MOVREL*): MoltenVK lowers a 512-entry private array to thread memory, and every + // register access of every invocation then spills through it. + private readonly Dictionary _scalarRegisterVariables = new(); + private readonly Dictionary _vectorRegisterVariables = new(); private uint _vectorRegisters; private uint _packedHalfRegisters; private uint _scc; @@ -539,20 +544,15 @@ public static partial class Gen5SpirvTranslator _privateBoolPointer = _module.TypePointer(SpirvStorageClass.Private, _boolType); - var scalarArrayType = _module.TypeArray(_uintType, ScalarRegisterCount); - var vectorArrayType = _module.TypeArray(_uintType, VectorRegisterCount); - var privateScalarArrayPointer = - _module.TypePointer(SpirvStorageClass.Private, scalarArrayType); - var privateVectorArrayPointer = - _module.TypePointer(SpirvStorageClass.Private, vectorArrayType); - _scalarRegisters = _module.AddGlobalVariable( - privateScalarArrayPointer, - SpirvStorageClass.Private, - _module.ConstantNull(scalarArrayType)); - _vectorRegisters = _module.AddGlobalVariable( - privateVectorArrayPointer, - SpirvStorageClass.Private, - _module.ConstantNull(vectorArrayType)); + if (_request.Program.Instructions.Any(static instruction => instruction.Opcode.StartsWith("VMovrel", StringComparison.Ordinal))) + { + var vectorArrayType = _module.TypeArray(_uintType, VectorRegisterCount); + _vectorRegisters = _module.AddGlobalVariable( + _module.TypePointer(SpirvStorageClass.Private, vectorArrayType), + SpirvStorageClass.Private, + _module.ConstantNull(vectorArrayType)); + } + _scc = _module.AddGlobalVariable( _privateBoolPointer, SpirvStorageClass.Private, @@ -594,8 +594,12 @@ public static partial class Gen5SpirvTranslator _module.AddName(_iterationGuard, "pcGuard"); } - _interfaces.Add(_scalarRegisters); - _interfaces.Add(_vectorRegisters); + if (_vectorRegisters != 0) + { + _interfaces.Add(_vectorRegisters); + _module.AddName(_vectorRegisters, "vgpr"); + } + _interfaces.Add(_scc); _interfaces.Add(_vcc); _interfaces.Add(_exec); @@ -607,8 +611,6 @@ public static partial class Gen5SpirvTranslator } _interfaces.Add(_programCounter); _interfaces.Add(_programActive); - _module.AddName(_scalarRegisters, "sgpr"); - _module.AddName(_vectorRegisters, "vgpr"); { DeclareLayoutBindings(); @@ -6750,12 +6752,28 @@ public static partial class Gen5SpirvTranslator UInt(0), dwordAddress); - private uint ScalarPointer(uint register) => - _module.AddInstruction( - SpirvOp.AccessChain, - _privateUintPointer, - _scalarRegisters, - UInt(register)); + private uint ScalarPointer(uint register) + { + if (register >= ScalarRegisterCount) + { + throw new InvalidOperationException($"Scalar register s{register} is outside the register file."); + } + + return RegisterVariable(_scalarRegisterVariables, register, "s"); + } + + private uint RegisterVariable(Dictionary variables, uint register, string prefix) + { + if (!variables.TryGetValue(register, out var variable)) + { + variable = _module.AddGlobalVariable(_privateUintPointer, SpirvStorageClass.Private, _module.ConstantNull(_uintType)); + _interfaces.Add(variable); + _module.AddName(variable, $"{prefix}{register}"); + variables.Add(register, variable); + } + + return variable; + } private uint RuntimeBufferBiasPointer(int binding) => _module.AddInstruction( @@ -6764,12 +6782,24 @@ public static partial class Gen5SpirvTranslator _runtimeBufferBiases, UInt(checked((uint)binding))); - private uint VectorPointer(uint register) => - _module.AddInstruction( - SpirvOp.AccessChain, - _privateUintPointer, - _vectorRegisters, - UInt(register)); + private uint VectorPointer(uint register) + { + if (_vectorRegisters != 0) + { + return _module.AddInstruction( + SpirvOp.AccessChain, + _privateUintPointer, + _vectorRegisters, + UInt(register)); + } + + if (register >= VectorRegisterCount) + { + throw new InvalidOperationException($"Vector register v{register} is outside the register file."); + } + + return RegisterVariable(_vectorRegisterVariables, register, "v"); + } // The V_MOVREL* opcodes address the VGPR file with a register number that // is only known at run time (encoded number + M0), so the access chain diff --git a/tests/SharpEmu.ShaderCompiler.Tests/Gen5ImageTests.cs b/tests/SharpEmu.ShaderCompiler.Tests/Gen5ImageTests.cs index eee1963..bf29bdf 100644 --- a/tests/SharpEmu.ShaderCompiler.Tests/Gen5ImageTests.cs +++ b/tests/SharpEmu.ShaderCompiler.Tests/Gen5ImageTests.cs @@ -316,20 +316,31 @@ public sealed class Gen5ImageTests } Assert.Equal(SpirvOp.Load, value.Opcode); - var pointer = FindResult( - instructions, - SpirvOp.AccessChain, - value.Operands[2]); - var register = FindResult( - instructions, - SpirvOp.Constant, - pointer.Operands[^1]); - result[component] = checked((int)register.Operands[2]); + result[component] = VgprIndexOfPointer(instructions, value.Operands[2]); } return result; } + // A VGPR is either its own Private variable named "v" or an access chain into the register array. + private static int VgprIndexOfPointer(IReadOnlyList instructions, uint pointerId) + { + var names = instructions.Where(item => + item.Opcode == SpirvOp.Name && item.Operands.Length > 1 && item.Operands[0] == pointerId).ToArray(); + if (names.Length == 1) + { + var name = names[0]; + var bytes = name.Operands.Skip(1).SelectMany(BitConverter.GetBytes).TakeWhile(static b => b != 0).ToArray(); + var text = System.Text.Encoding.UTF8.GetString(bytes); + Assert.StartsWith("v", text); + return int.Parse(text[1..], System.Globalization.CultureInfo.InvariantCulture); + } + + var pointer = FindResult(instructions, SpirvOp.AccessChain, pointerId); + var register = FindResult(instructions, SpirvOp.Constant, pointer.Operands[^1]); + return checked((int)register.Operands[2]); + } + private static ParsedSpirvInstruction FindValueDefinition( IReadOnlyList instructions, uint resultId) => From c13e09130cb06f771b2aeee644edbf91a1867c7f Mon Sep 17 00:00:00 2001 From: kuba Date: Sun, 27 Sep 2026 16:18:52 +0200 Subject: [PATCH 2/2] [ShaderCompiler] Resolve V_MOVREL* with selects instead of a spilled VGPR array A program with any V_MOVREL* instruction declared its whole VGPR file as a 512-entry private array indexed at run time, which Metal spills to thread memory, so every register access became a memory access. Keep one variable per register and resolve relative accesses with a select over the registers the program names. Octopath's instanced vertex shader (1536 x 8124): 112 ms -> 6.4 ms per draw in replay, output byte-identical. --- .../Gen5SpirvTranslator.cs | 78 ++++++++++++++----- .../Agc/Gen5MoveRelativeSpirvTests.cs | 73 ++++++----------- 2 files changed, 81 insertions(+), 70 deletions(-) diff --git a/src/SharpEmu.ShaderCompiler.Vulkan/Gen5SpirvTranslator.cs b/src/SharpEmu.ShaderCompiler.Vulkan/Gen5SpirvTranslator.cs index a9082ab..9da55ea 100644 --- a/src/SharpEmu.ShaderCompiler.Vulkan/Gen5SpirvTranslator.cs +++ b/src/SharpEmu.ShaderCompiler.Vulkan/Gen5SpirvTranslator.cs @@ -151,6 +151,7 @@ public static partial class Gen5SpirvTranslator private readonly Dictionary _scalarRegisterVariables = new(); private readonly Dictionary _vectorRegisterVariables = new(); private uint _vectorRegisters; + private (uint First, uint Last) _dynamicVectorRange; private uint _packedHalfRegisters; private uint _scc; private uint _vcc; @@ -546,11 +547,11 @@ public static partial class Gen5SpirvTranslator if (_request.Program.Instructions.Any(static instruction => instruction.Opcode.StartsWith("VMovrel", StringComparison.Ordinal))) { - var vectorArrayType = _module.TypeArray(_uintType, VectorRegisterCount); - _vectorRegisters = _module.AddGlobalVariable( - _module.TypePointer(SpirvStorageClass.Private, vectorArrayType), - SpirvStorageClass.Private, - _module.ConstantNull(vectorArrayType)); + // A dynamically indexed private array cannot live in registers: Metal spills the + // whole 2 KiB file to thread memory and every VGPR access becomes a memory access + // (Octopath's instanced vertex shader: 112 ms per draw). Keep one variable per + // register and resolve V_MOVREL* with a select over the registers the program uses. + _dynamicVectorRange = DynamicVectorRange(_request.Program); } _scc = _module.AddGlobalVariable( @@ -6806,26 +6807,61 @@ public static partial class Gen5SpirvTranslator // takes a computed index instead of a constant. The index is masked to // the array bounds: SPIR-V leaves an out-of-range Private access chain // undefined, and a mask costs nothing next to the surrounding load. - private uint DynamicVectorPointer(uint registerIndex) => - _module.AddInstruction( - SpirvOp.AccessChain, - _privateUintPointer, - _vectorRegisters, - BitwiseAnd(registerIndex, UInt(VectorRegisterCount - 1))); + // Every register a relative access can reach: from the lowest V_MOVREL* base to past the + // highest register the program names (a multi-dword result may extend beyond its listed + // first register). An index outside it reads zero and writes nothing. + private static (uint First, uint Last) DynamicVectorRange(Gen5ShaderProgram program) + { + var first = VectorRegisterCount - 1; + var last = 0u; + foreach (var instruction in program.Instructions) + { + foreach (var operand in instruction.Sources.Concat(instruction.Destinations)) + { + if (operand.Kind == Gen5OperandKind.VectorRegister) + { + last = Math.Max(last, operand.Value); + if (instruction.Opcode.StartsWith("VMovrel", StringComparison.Ordinal)) + { + first = Math.Min(first, operand.Value); + } + } + } + } - private uint LoadVDynamic(uint registerIndex) => - Load(_uintType, DynamicVectorPointer(registerIndex)); + last = Math.Min(last + 8, VectorRegisterCount - 1); + return (Math.Min(first, last), last); + } + + private uint LoadVDynamic(uint registerIndex) + { + var result = UInt(0); + for (var register = _dynamicVectorRange.First; register <= _dynamicVectorRange.Last; register++) + { + result = _module.AddInstruction( + SpirvOp.Select, + _uintType, + _module.AddInstruction(SpirvOp.IEqual, _boolType, registerIndex, UInt(register)), + Load(_uintType, VectorPointer(register)), + result); + } + + return result; + } private void StoreVDynamic(uint registerIndex, uint value) { - var pointer = DynamicVectorPointer(registerIndex); - value = _module.AddInstruction( - SpirvOp.Select, - _uintType, - Load(_boolType, _exec), - value, - Load(_uintType, pointer)); - Store(pointer, value); + var exec = Load(_boolType, _exec); + for (var register = _dynamicVectorRange.First; register <= _dynamicVectorRange.Last; register++) + { + var pointer = VectorPointer(register); + var selected = _module.AddInstruction( + SpirvOp.LogicalAnd, + _boolType, + exec, + _module.AddInstruction(SpirvOp.IEqual, _boolType, registerIndex, UInt(register))); + Store(pointer, _module.AddInstruction(SpirvOp.Select, _uintType, selected, value, Load(_uintType, pointer))); + } } private uint PackedHalfPointer(uint register) => diff --git a/tests/SharpEmu.Libs.Tests/Agc/Gen5MoveRelativeSpirvTests.cs b/tests/SharpEmu.Libs.Tests/Agc/Gen5MoveRelativeSpirvTests.cs index bf81482..b36955b 100644 --- a/tests/SharpEmu.Libs.Tests/Agc/Gen5MoveRelativeSpirvTests.cs +++ b/tests/SharpEmu.Libs.Tests/Agc/Gen5MoveRelativeSpirvTests.cs @@ -10,7 +10,8 @@ using Xunit; namespace SharpEmu.Libs.Tests.Agc; -// Checks computed register indices and rejects invalid relative sources. +// Checks that relative register accesses resolve without a dynamically indexed register +// file, and rejects invalid relative sources. public sealed class Gen5MoveRelativeSpirvTests { private const ulong ShaderAddress = 0x1_0000_0000; @@ -24,25 +25,21 @@ public sealed class Gen5MoveRelativeSpirvTests private const uint SMovM0 = 0xBE800000u | (124u << 16) | (0x03u << 8) | 130u; [Fact] - public void MovrelsB32_ReadsTheSourceRegisterThroughADynamicIndex() + public void MovrelsB32_ReadsTheSourceRegisterThroughAComputedIndex() { // s_mov_b32 m0, 2 ; v_movrels_b32 v5, v3 -> v5 = vgpr[3 + m0] var spirv = Compile([SMovM0, Vop1 | (5u << 17) | (0x43u << 9) | (256u + 3u)]); - Assert.True( - HasDynamicVectorRegisterAccess(spirv), - "V_MOVRELS_B32 must index the VGPR array with a computed index"); + AssertResolvesRelativeRegistersWithSelects(spirv, "V_MOVRELS_B32"); } [Fact] - public void MovreldB32_WritesTheDestinationRegisterThroughADynamicIndex() + public void MovreldB32_WritesTheDestinationRegisterThroughAComputedIndex() { // s_mov_b32 m0, 2 ; v_movreld_b32 v5, v3 -> vgpr[5 + m0] = v3 var spirv = Compile([SMovM0, Vop1 | (5u << 17) | (0x42u << 9) | (256u + 3u)]); - Assert.True( - HasDynamicVectorRegisterAccess(spirv), - "V_MOVRELD_B32 must index the VGPR array with a computed index"); + AssertResolvesRelativeRegistersWithSelects(spirv, "V_MOVRELD_B32"); } [Fact] @@ -51,9 +48,7 @@ public sealed class Gen5MoveRelativeSpirvTests // s_mov_b32 m0, 2 ; v_movrelsd_b32 v5, v3 -> vgpr[5 + m0] = vgpr[3 + m0] var spirv = Compile([SMovM0, Vop1 | (5u << 17) | (0x44u << 9) | (256u + 3u)]); - Assert.True( - HasDynamicVectorRegisterAccess(spirv), - "V_MOVRELSD_B32 must index the VGPR array with a computed index"); + AssertResolvesRelativeRegistersWithSelects(spirv, "V_MOVRELSD_B32"); } [Fact] @@ -63,9 +58,7 @@ public sealed class Gen5MoveRelativeSpirvTests // 10-bit halves (source index in [9:0], destination index in [25:16]). var spirv = Compile([SMovM0, Vop1 | (5u << 17) | (0x48u << 9) | (256u + 3u)]); - Assert.True( - HasDynamicVectorRegisterAccess(spirv), - "V_MOVRELSD_2_B32 must index the VGPR array with a computed index"); + AssertResolvesRelativeRegistersWithSelects(spirv, "V_MOVRELSD_2_B32"); } [Fact] @@ -82,14 +75,14 @@ public sealed class Gen5MoveRelativeSpirvTests Assert.Contains("vector register", error, StringComparison.Ordinal); } - // True when some OpAccessChain into the "vgpr" array uses an index that is - // not an OpConstant — i.e. a register number computed from M0. - private static bool HasDynamicVectorRegisterAccess(byte[] spirv) + // A dynamically indexed private register array is lowered to thread memory on Metal, so no + // access chain may take a computed index; the relative access compares the computed register + // number with each candidate register (OpIEqual) and selects its value (OpSelect). + private static void AssertResolvesRelativeRegistersWithSelects(byte[] spirv, string opcode) { - var vectorRegisters = FindNamedId(spirv, "vgpr"); - Assert.True(vectorRegisters != 0, "the module must name its VGPR array"); - var constants = new HashSet(); + var equalities = 0; + var selects = 0; foreach (var (op, wordCount, offset) in EnumerateInstructions(spirv)) { // OpConstant = 43, OpConstantNull = 46: (opcode, resultType, resultId, ...). @@ -97,47 +90,29 @@ public sealed class Gen5MoveRelativeSpirvTests { constants.Add(ReadWord(spirv, offset + 8)); } + + // OpIEqual = 170, OpSelect = 169. + equalities += op == 170 ? 1 : 0; + selects += op == 169 ? 1 : 0; } foreach (var (op, wordCount, offset) in EnumerateInstructions(spirv)) { // OpAccessChain = 65: (opcode, resultType, resultId, base, index...). - if (op != 65 || wordCount < 5 || ReadWord(spirv, offset + 12) != vectorRegisters) + if (op != 65) { continue; } - if (!constants.Contains(ReadWord(spirv, offset + 16))) + for (var index = 4; index < wordCount; index++) { - return true; + Assert.True( + constants.Contains(ReadWord(spirv, offset + index * sizeof(uint))), + $"{opcode} must not index a register array with a computed index"); } } - return false; - } - - // Result id of the OpName whose literal string matches, or 0. - private static uint FindNamedId(byte[] spirv, string name) - { - foreach (var (op, wordCount, offset) in EnumerateInstructions(spirv)) - { - // OpName = 5: (opcode, target, literal string...). - if (op != 5 || wordCount < 3) - { - continue; - } - - var bytes = spirv.AsSpan(offset + 8, (wordCount - 2) * sizeof(uint)); - var terminator = bytes.IndexOf((byte)0); - var text = System.Text.Encoding.UTF8.GetString( - terminator < 0 ? bytes : bytes[..terminator]); - if (text == name) - { - return ReadWord(spirv, offset + 4); - } - } - - return 0; + Assert.True(equalities > 1 && selects > 1, $"{opcode} must select the register its computed index names"); } private static IEnumerable<(ushort Op, int WordCount, int Offset)> EnumerateInstructions(