From b6567ced0c3995d40e7d9b5ad08e208dfba52db3 Mon Sep 17 00:00:00 2001 From: Akeit0 <90429982+Akeit0@users.noreply.github.com> Date: Fri, 7 Mar 2025 20:53:01 +0900 Subject: [PATCH 1/3] Fix: `or` doesn't work well --- .../Compilation/FunctionCompilationContext.cs | 57 +++++++++++-------- .../CodeAnalysis/Compilation/LuaCompiler.cs | 14 ++++- .../Compilation/ScopeCompilationContext.cs | 8 +-- src/Lua/Internal/BitFlags256.cs | 23 ++++++++ tests/Lua.Tests/ConditionalsTests.cs | 22 +++++++ 5 files changed, 95 insertions(+), 29 deletions(-) create mode 100644 src/Lua/Internal/BitFlags256.cs create mode 100644 tests/Lua.Tests/ConditionalsTests.cs diff --git a/src/Lua/CodeAnalysis/Compilation/FunctionCompilationContext.cs b/src/Lua/CodeAnalysis/Compilation/FunctionCompilationContext.cs index e564f660..c67ee5cf 100644 --- a/src/Lua/CodeAnalysis/Compilation/FunctionCompilationContext.cs +++ b/src/Lua/CodeAnalysis/Compilation/FunctionCompilationContext.cs @@ -119,7 +119,7 @@ public void PushInstruction(in Instruction instruction, in SourcePosition positi /// Push or merge the new instruction. /// [MethodImpl(MethodImplOptions.AggressiveInlining)] - public void PushOrMergeInstruction(int lastLocal, in Instruction instruction, in SourcePosition position, ref bool incrementStackPosition) + internal void PushOrMergeInstruction(in Instruction instruction, in SourcePosition position, ref bool incrementStackPosition) { if (instructions.Length == 0) { @@ -127,27 +127,41 @@ public void PushOrMergeInstruction(int lastLocal, in Instruction instruction, in instructionPositions.Add(position); return; } + + var activeLocals = Scope.ActiveLocalVariables; + ref var lastInstruction = ref instructions.AsSpan()[^1]; var opcode = instruction.OpCode; switch (opcode) { case OpCode.Move: - // last A is not local variable - if (lastInstruction.A != lastLocal && - // available to merge - lastInstruction.A == instruction.B && - // not already merged - lastInstruction.A != lastInstruction.B) + + if ( + // available to merge and last A is not local variable + lastInstruction.A == instruction.B && !activeLocals[lastInstruction.A]) { switch (lastInstruction.OpCode) { + case OpCode.LoadK: + case OpCode.LoadBool when lastInstruction.C == 0: + case OpCode.LoadNil when lastInstruction.B == 0: + case OpCode.GetUpVal: + case OpCode.GetTabUp: case OpCode.GetTable: + case OpCode.SetTabUp: + case OpCode.SetUpVal: + case OpCode.SetTable: + case OpCode.NewTable: + case OpCode.Self: case OpCode.Add: case OpCode.Sub: case OpCode.Mul: case OpCode.Div: case OpCode.Mod: case OpCode.Pow: + case OpCode.Unm: + case OpCode.Not: + case OpCode.Len: case OpCode.Concat: { lastInstruction.A = instruction.A; @@ -156,11 +170,12 @@ public void PushOrMergeInstruction(int lastLocal, in Instruction instruction, in } } } + break; case OpCode.GetTable: { // Merge MOVE GetTable - if (lastInstruction.OpCode == OpCode.Move && lastLocal != lastInstruction.A) + if (lastInstruction.OpCode == OpCode.Move && !activeLocals[lastInstruction.A]) { if (lastInstruction.A == instruction.B) { @@ -169,14 +184,14 @@ public void PushOrMergeInstruction(int lastLocal, in Instruction instruction, in incrementStackPosition = false; return; } - } + break; } case OpCode.SetTable: { // Merge MOVE SETTABLE - if (lastInstruction.OpCode == OpCode.Move && lastLocal != lastInstruction.A) + if (lastInstruction.OpCode == OpCode.Move && !activeLocals[lastInstruction.A]) { var lastB = lastInstruction.B; var lastA = lastInstruction.A; @@ -187,7 +202,7 @@ public void PushOrMergeInstruction(int lastLocal, in Instruction instruction, in { ref var last2Instruction = ref instructions.AsSpan()[^2]; var last2A = last2Instruction.A; - if (last2Instruction.OpCode == OpCode.Move && lastLocal != last2A && instruction.C == last2A) + if (last2Instruction.OpCode == OpCode.Move && !activeLocals[last2A] && instruction.C == last2A) { last2Instruction = Instruction.SetTable((byte)(lastB), instruction.B, last2Instruction.B); instructions.RemoveAtSwapback(instructions.Length - 1); @@ -197,6 +212,7 @@ public void PushOrMergeInstruction(int lastLocal, in Instruction instruction, in return; } } + lastInstruction = Instruction.SetTable((byte)(lastB), instruction.B, instruction.C); instructionPositions[^1] = position; incrementStackPosition = false; @@ -217,9 +233,8 @@ public void PushOrMergeInstruction(int lastLocal, in Instruction instruction, in var last2OpCode = last2Instruction.OpCode; if (last2OpCode is OpCode.LoadK or OpCode.Move) { - var last2A = last2Instruction.A; - if (last2A != lastLocal && instruction.C == last2A) + if (!activeLocals[last2A] && instruction.C == last2A) { var c = last2OpCode == OpCode.LoadK ? last2Instruction.Bx + 256 : last2Instruction.B; last2Instruction = lastInstruction; @@ -231,27 +246,20 @@ public void PushOrMergeInstruction(int lastLocal, in Instruction instruction, in } } } + break; } case OpCode.Unm: case OpCode.Not: case OpCode.Len: - if (lastInstruction.OpCode == OpCode.Move && lastLocal != lastInstruction.A && lastInstruction.A == instruction.B) + if (lastInstruction.OpCode == OpCode.Move && !activeLocals[lastInstruction.A] && lastInstruction.A == instruction.B) { - lastInstruction = instruction with { B = lastInstruction.B }; ; - instructionPositions[^1] = position; - incrementStackPosition = false; - return; - } - break; - case OpCode.Return: - if (lastInstruction.OpCode == OpCode.Move && instruction.B == 2 && lastInstruction.B < 256) - { - lastInstruction = instruction with { A = (byte)lastInstruction.B }; + lastInstruction = instruction with { B = lastInstruction.B }; instructionPositions[^1] = position; incrementStackPosition = false; return; } + break; } @@ -378,6 +386,7 @@ public void ResolveAllBreaks(byte startPosition, int endPosition, ScopeCompilati { instruction.A = startPosition; } + instruction.SBx = endPosition - description.Index; } diff --git a/src/Lua/CodeAnalysis/Compilation/LuaCompiler.cs b/src/Lua/CodeAnalysis/Compilation/LuaCompiler.cs index 3cd17910..ebff88e0 100644 --- a/src/Lua/CodeAnalysis/Compilation/LuaCompiler.cs +++ b/src/Lua/CodeAnalysis/Compilation/LuaCompiler.cs @@ -144,7 +144,13 @@ public bool VisitBinaryExpressionNode(BinaryExpressionNode node, ScopeCompilatio a = context.StackTopPosition; } - context.PushInstruction(Instruction.Test(a, (byte)(node.OperatorType is BinaryOperator.And ? 0 : 1)), node.Position); + context.PushInstruction(Instruction.Test(a, 0), node.Position); + if (node.OperatorType is BinaryOperator.Or) + { + context.PushInstruction(Instruction.Jmp(0, 2), node.Position); + context.PushInstruction(Instruction.Move(r, a), node.Position); + } + var testJmpIndex = context.Function.Instructions.Length; context.PushInstruction(Instruction.Jmp(0, 0), node.Position); @@ -469,6 +475,7 @@ public bool VisitLocalAssignmentStatementNode(LocalAssignmentStatementNode node, }); } } + return true; } @@ -1102,6 +1109,7 @@ static bool TryGetConstant(ExpressionNode node, ScopeCompilationContext context, { value = stringLiteral.Text.ToString(); } + return true; case UnaryExpressionNode unaryExpression: if (TryGetConstant(unaryExpression.Node, context, out var unaryNodeValue)) @@ -1114,6 +1122,7 @@ static bool TryGetConstant(ExpressionNode node, ScopeCompilationContext context, value = -d1; return true; } + break; case UnaryOperator.Not: if (unaryNodeValue.TryRead(out var b)) @@ -1121,9 +1130,11 @@ static bool TryGetConstant(ExpressionNode node, ScopeCompilationContext context, value = !b; return true; } + break; } } + break; case BinaryExpressionNode binaryExpression: if (TryGetConstant(binaryExpression.LeftNode, context, out var leftValue) && @@ -1169,6 +1180,7 @@ static bool TryGetConstant(ExpressionNode node, ScopeCompilationContext context, break; } } + break; } diff --git a/src/Lua/CodeAnalysis/Compilation/ScopeCompilationContext.cs b/src/Lua/CodeAnalysis/Compilation/ScopeCompilationContext.cs index 51c67984..6dc0e533 100644 --- a/src/Lua/CodeAnalysis/Compilation/ScopeCompilationContext.cs +++ b/src/Lua/CodeAnalysis/Compilation/ScopeCompilationContext.cs @@ -33,7 +33,7 @@ public static void Return(ScopeCompilationContext context) readonly Dictionary, LocalVariableDescription> localVariables = new(256, Utf16StringMemoryComparer.Default); readonly Dictionary, LabelDescription> labels = new(32, Utf16StringMemoryComparer.Default); - byte lastLocalVariableIndex; + internal BitFlags256 ActiveLocalVariables = default; public byte StackStartPosition { get; private set; } public byte StackPosition { get; set; } @@ -74,7 +74,7 @@ public FunctionCompilationContext CreateChildFunction() [MethodImpl(MethodImplOptions.AggressiveInlining)] public void PushInstruction(in Instruction instruction, SourcePosition position, bool incrementStackPosition = false) { - Function.PushOrMergeInstruction(lastLocalVariableIndex, instruction, position, ref incrementStackPosition); + Function.PushOrMergeInstruction(instruction, position, ref incrementStackPosition); if (incrementStackPosition) { StackPosition++; @@ -98,7 +98,7 @@ public void TryPushCloseUpValue(byte top, SourcePosition position) public void AddLocalVariable(ReadOnlyMemory name, LocalVariableDescription description, bool markAsLastLocalVariable = true) { localVariables[name] = description; - lastLocalVariableIndex = description.RegisterIndex; + ActiveLocalVariables.Set(description.RegisterIndex); } @@ -165,7 +165,7 @@ public void Reset() HasCapturedLocalVariables = false; localVariables.Clear(); labels.Clear(); - lastLocalVariableIndex = 0; + ActiveLocalVariables = default; } /// diff --git a/src/Lua/Internal/BitFlags256.cs b/src/Lua/Internal/BitFlags256.cs new file mode 100644 index 00000000..0e3a3176 --- /dev/null +++ b/src/Lua/Internal/BitFlags256.cs @@ -0,0 +1,23 @@ +namespace Lua.Internal; + +internal unsafe struct BitFlags256 +{ + internal fixed long Data[4]; + + public bool this[int index] + { + get => (Data[index >> 6] & (1L << (index & 63))) != 0; + set + { + if (value) + { + Data[index >> 6] |= 1L << (index & 63); + } + else + { + Data[index >> 6] &= ~(1L << (index & 63)); + } + } + } + public void Set(int index) => Data[index >> 6] |= 1L << (index & 63); +} \ No newline at end of file diff --git a/tests/Lua.Tests/ConditionalsTests.cs b/tests/Lua.Tests/ConditionalsTests.cs new file mode 100644 index 00000000..3bec16da --- /dev/null +++ b/tests/Lua.Tests/ConditionalsTests.cs @@ -0,0 +1,22 @@ +namespace Lua.Tests; + +public class ConditionalsTests +{ + [Test] + public async Task Test_Clamp() + { + var source = @" +function clamp(x, min, max) + return x < min and min or (x > max and max or x) +end + +return clamp(0, 1, 25), clamp(10, 1, 25), clamp(30, 1, 25) +"; + var result = await LuaState.Create().DoStringAsync(source); + + Assert.That(result, Has.Length.EqualTo(3)); + Assert.That(result[0], Is.EqualTo(new LuaValue(1))); + Assert.That(result[1], Is.EqualTo(new LuaValue(10))); + Assert.That(result[2], Is.EqualTo(new LuaValue(25))); + } +} \ No newline at end of file From d54cca6432d62f7c64e19367164c3b1a6430465c Mon Sep 17 00:00:00 2001 From: Akeit0 <90429982+Akeit0@users.noreply.github.com> Date: Fri, 7 Mar 2025 20:55:44 +0900 Subject: [PATCH 2/3] Fix: revert access modifier --- src/Lua/CodeAnalysis/Compilation/FunctionCompilationContext.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Lua/CodeAnalysis/Compilation/FunctionCompilationContext.cs b/src/Lua/CodeAnalysis/Compilation/FunctionCompilationContext.cs index c67ee5cf..7128e898 100644 --- a/src/Lua/CodeAnalysis/Compilation/FunctionCompilationContext.cs +++ b/src/Lua/CodeAnalysis/Compilation/FunctionCompilationContext.cs @@ -119,7 +119,7 @@ public void PushInstruction(in Instruction instruction, in SourcePosition positi /// Push or merge the new instruction. /// [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal void PushOrMergeInstruction(in Instruction instruction, in SourcePosition position, ref bool incrementStackPosition) + public void PushOrMergeInstruction(in Instruction instruction, in SourcePosition position, ref bool incrementStackPosition) { if (instructions.Length == 0) { From 903068d74e324f689c07935b3ad2b2a360a4b045 Mon Sep 17 00:00:00 2001 From: Akeit0 <90429982+Akeit0@users.noreply.github.com> Date: Wed, 16 Apr 2025 23:32:40 +0900 Subject: [PATCH 3/3] Fix: unintended InstructionMerge of GetTable --- src/Lua/CodeAnalysis/Compilation/FunctionCompilationContext.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Lua/CodeAnalysis/Compilation/FunctionCompilationContext.cs b/src/Lua/CodeAnalysis/Compilation/FunctionCompilationContext.cs index 7128e898..ef0c6a0c 100644 --- a/src/Lua/CodeAnalysis/Compilation/FunctionCompilationContext.cs +++ b/src/Lua/CodeAnalysis/Compilation/FunctionCompilationContext.cs @@ -147,7 +147,7 @@ public void PushOrMergeInstruction(in Instruction instruction, in SourcePosition case OpCode.LoadNil when lastInstruction.B == 0: case OpCode.GetUpVal: case OpCode.GetTabUp: - case OpCode.GetTable: + case OpCode.GetTable when !activeLocals[lastInstruction.B]: case OpCode.SetTabUp: case OpCode.SetUpVal: case OpCode.SetTable: