Fixes #130: Undefined function return value

Added function return value as an output argument to CALL/CALLREC

doc/syntax/SYNTAX-4-FUNCTIONS.markdown

@@ -109,9 +109,9 @@ Again, the `vault1` or `storage` in the examples can be a variable or a linked b
 A `sync` instruction (available in Mindustry Logic since version 7.0 build 146) is mapped to a `sync()` function. 
 The function has one parameter - a variable to be synchronized across the network (namely, from the server to all 
 clients). A [global variable](SYNTAX-1-VARIABLES.markdown#global-variables) must be passed as an argument to this 
-function, otherwise a compile error occurs. The reason is that global variables are more restrained in optimizations 
-and therefore their dataflow is less altered. If local variables were used, they might not contain the expected 
-value, as some (or all) assignments to them can be eliminated by the Data Flow Optimization.
+function, otherwise a compilation error occurs. The reason is that global variables are more restrained in 
+optimizations and therefore their dataflow is less altered. If local variables were used, they might not contain the 
+expected value, as some (or all) assignments to them can be eliminated by the Data Flow Optimization.
 
 This constraint makes sense semantically as well: a scope of a global variable is the entire program. When a 
 variable is synced, its scope becomes even broader and is shared between multiple processors; using a local variable 

mindcode/src/main/java/info/teksol/mindcode/compiler/generator/LogicInstructionGenerator.java

@@ -174,8 +174,8 @@ public LogicVariable visitVariable(AstNode node) {
     }
 
 
-    public CallRecInstruction createCallRecursive(LogicVariable stack, LogicLabel callAddr, LogicLabel retAddr) {
-        return instructionProcessor.createCallRecursive(astContext, stack, callAddr, retAddr);
+    public CallRecInstruction createCallRecursive(LogicVariable stack, LogicLabel callAddr, LogicLabel retAddr, LogicVariable returnValue) {
+        return instructionProcessor.createCallRecursive(astContext, stack, callAddr, retAddr, returnValue);
     }
 
     public EndInstruction createEnd() {
@@ -251,8 +251,8 @@ public SetAddressInstruction createSetAddress(LogicVariable variable, LogicLabel
         return instructionProcessor.createSetAddress(astContext, variable, address);
     }
 
-    public CallInstruction createCallStackless(LogicAddress value) {
-        return instructionProcessor.createCallStackless(astContext, value);
+    public CallInstruction createCallStackless(LogicAddress value, LogicVariable returnValue) {
+        return instructionProcessor.createCallStackless(astContext, value, returnValue);
     }
 
     public StopInstruction createStop() {
@@ -466,7 +466,7 @@ private LogicValue handleStacklessFunctionCall(Function function, List<LogicValu
         setSubcontextType(function, AstSubcontextType.OUT_OF_LINE_CALL);
         final LogicLabel returnLabel = nextLabel();
         emit(createSetAddress(LogicVariable.fnRetAddr(functionPrefix), returnLabel));
-        emit(createCallStackless(function.getLabel()));
+        emit(createCallStackless(function.getLabel(), LogicVariable.fnRetVal(functionPrefix)));
         // Mark position where the function must return
         // TODO (STACKLESS_CALL) We no longer need to track relationship between return from the stackless call and callee
         //      Use GOTO_OFFSET for list iterator, drop marker from GOTO and target simple labels
@@ -495,7 +495,8 @@ private LogicValue handleRecursiveFunctionCall(Function function, List<LogicValu
 
          // Recursive function call
         final LogicLabel returnLabel = nextLabel();
-        emit(createCallRecursive(stackName(), function.getLabel(), returnLabel));
+        emit(createCallRecursive(stackName(), function.getLabel(), returnLabel,
+                LogicVariable.fnRetVal(functionPrefix)));
         emit(createLabel(returnLabel)); // where the function must return
 
         if (useStack) {

mindcode/src/main/java/info/teksol/mindcode/compiler/instructions/BaseInstructionProcessor.java

@@ -94,13 +94,13 @@ public LogicInstruction fromOpcodeVariant(OpcodeVariant opcodeVariant) {
     }
 
     @Override
-    public CallInstruction createCallStackless(AstContext astContext, LogicAddress address) {
-        return (CallInstruction) createInstruction(astContext, CALL, address);
+    public CallInstruction createCallStackless(AstContext astContext, LogicAddress address, LogicVariable returnValue) {
+        return (CallInstruction) createInstruction(astContext, CALL, address, returnValue);
     }
 
     @Override
-    public CallRecInstruction createCallRecursive(AstContext astContext, LogicVariable stack, LogicLabel callAddr, LogicLabel retAddr) {
-        return (CallRecInstruction) createInstruction(astContext, CALLREC, stack, callAddr, retAddr);
+    public CallRecInstruction createCallRecursive(AstContext astContext, LogicVariable stack, LogicLabel callAddr, LogicLabel retAddr, LogicVariable returnValue) {
+        return (CallRecInstruction) createInstruction(astContext, CALLREC, stack, callAddr, retAddr, returnValue);
     }
 
     @Override

mindcode/src/main/java/info/teksol/mindcode/compiler/instructions/CallInstruction.java

@@ -1,10 +1,7 @@
 package info.teksol.mindcode.compiler.instructions;
 
 import info.teksol.mindcode.compiler.generator.AstContext;
-import info.teksol.mindcode.logic.LogicArgument;
-import info.teksol.mindcode.logic.LogicLabel;
-import info.teksol.mindcode.logic.LogicParameter;
-import info.teksol.mindcode.logic.Opcode;
+import info.teksol.mindcode.logic.*;
 
 import java.util.List;
 
@@ -31,4 +28,8 @@ public CallInstruction withContext(AstContext astContext) {
     public final LogicLabel getCallAddr() {
         return (LogicLabel) getArg(0);
     }
+
+    public final LogicVariable getReturnValue()  {
+        return (LogicVariable) getArg(1);
+    }
 }

mindcode/src/main/java/info/teksol/mindcode/compiler/instructions/InstructionProcessor.java

@@ -30,8 +30,8 @@ public interface InstructionProcessor {
     LogicInstruction fromOpcodeVariant(OpcodeVariant opcodeVariant);
 
 
-    CallInstruction createCallStackless(AstContext astContext, LogicAddress address);
-    CallRecInstruction createCallRecursive(AstContext astContext, LogicVariable stack, LogicLabel callAddr, LogicLabel retAddr);
+    CallInstruction createCallStackless(AstContext astContext, LogicAddress address, LogicVariable returnValue);
+    CallRecInstruction createCallRecursive(AstContext astContext, LogicVariable stack, LogicLabel callAddr, LogicLabel retAddr, LogicVariable returnValue);
     EndInstruction createEnd(AstContext astContext);
     GotoInstruction createGoto(AstContext astContext, LogicVariable address, LogicLabel marker);
     GotoOffsetInstruction createGotoOffset(AstContext astContext, LogicLabel target, LogicVariable value, LogicNumber offset, LogicLabel marker);
@@ -105,7 +105,7 @@ public interface InstructionProcessor {
      * arguments are the same. Side effects aren't considered. Values and character of the input arguments to the
      * instruction are considered - an otherwise deterministic instruction that has a volatile variable as an input
      * argument isn't deterministic.
-     *
+     * <p>
      * Note: getlink is not deemed a deterministic instruction, as it can be influenced by linking updating
      * blocks linked to the processor in the Mindustry World. Block variables are considered volatile
      * for the same reason.

mindcode/src/main/java/info/teksol/mindcode/compiler/optimization/AbstractOptimizer.java

@@ -90,12 +90,12 @@ protected boolean isVolatile(LogicInstruction instruction) {
     //</editor-fold>
 
     //<editor-fold desc="Instruction creation">
-    protected CallInstruction createCallStackless(AstContext astContext, LogicAddress address) {
-        return instructionProcessor.createCallStackless(astContext, address);
+    protected CallInstruction createCallStackless(AstContext astContext, LogicAddress address, LogicVariable returnValue) {
+        return instructionProcessor.createCallStackless(astContext, address, returnValue);
     }
 
-    protected CallRecInstruction createCallRecursive(AstContext astContext, LogicVariable stack, LogicLabel callAddr, LogicLabel retAddr) {
-        return instructionProcessor.createCallRecursive(astContext, stack, callAddr, retAddr);
+    protected CallRecInstruction createCallRecursive(AstContext astContext, LogicVariable stack, LogicLabel callAddr, LogicLabel retAddr, LogicVariable returnValue) {
+        return instructionProcessor.createCallRecursive(astContext, stack, callAddr, retAddr, returnValue);
     }
 
     protected EndInstruction createEnd(AstContext astContext) {

mindcode/src/main/java/info/teksol/mindcode/compiler/optimization/DeadCodeEliminator.java

@@ -5,6 +5,7 @@
 import info.teksol.mindcode.compiler.instructions.PushOrPopInstruction;
 import info.teksol.mindcode.logic.ArgumentType;
 import info.teksol.mindcode.logic.LogicVariable;
+import info.teksol.mindcode.logic.Opcode;
 
 import java.util.*;
 import java.util.function.Predicate;
@@ -116,10 +117,12 @@ private void examineInstruction(LogicInstruction instruction) {
                 .map(LogicVariable.class::cast)
                 .forEach(reads::add);
 
-        instruction.outputArgumentsStream()
-                .filter(LogicVariable.class::isInstance)
-                .map(LogicVariable.class::cast)
-                .filter(Predicate.not(LogicVariable::isCompilerVariable))
-                .forEach(v -> addWrite(instruction, v));
+        if (instruction.getOpcode() != Opcode.CALL && instruction.getOpcode() != Opcode.CALLREC) {
+            instruction.outputArgumentsStream()
+                    .filter(LogicVariable.class::isInstance)
+                    .map(LogicVariable.class::cast)
+                    .filter(Predicate.not(LogicVariable::isCompilerVariable))
+                    .forEach(v -> addWrite(instruction, v));
+        }
     }
 }

mindcode/src/main/java/info/teksol/mindcode/compiler/optimization/FunctionInliner.java

@@ -7,9 +7,7 @@
 import info.teksol.mindcode.compiler.instructions.EndInstruction;
 import info.teksol.mindcode.compiler.instructions.GotoInstruction;
 import info.teksol.mindcode.compiler.instructions.LogicInstruction;
-import info.teksol.mindcode.compiler.instructions.SetInstruction;
 import info.teksol.mindcode.compiler.optimization.OptimizationContext.LogicList;
-import info.teksol.mindcode.logic.LogicVariable;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -115,7 +113,7 @@ private OptimizationResult inlineFunction(AstContext context, int costLimit) {
             AstContext newContext = call.parent().createSubcontext(INLINE_CALL, 1.0);
             int insertionPoint = firstInstructionIndex(call);
             LogicList newBody = body.duplicateToContext(newContext);
-            insertInstructions(insertionPoint, swapReturnVariable(call, newBody));
+            insertInstructions(insertionPoint, newBody);
             // Remove original call instructions
             removeMatchingInstructions(ix -> ix.belongsTo(call));
         }
@@ -129,25 +127,6 @@ private OptimizationResult inlineFunction(AstContext context, int costLimit) {
         return OptimizationResult.REALIZED;
     }
 
-    private LogicList swapReturnVariable(AstContext call, LogicList newBody) {
-        LogicInstruction followup = instructionAfter(call);
-        if (followup instanceof SetInstruction set
-                && set.getValue() instanceof LogicVariable variable
-                && variable.getFunctionPrefix().equals(call.functionPrefix())) {
-            LogicVariable result = set.getResult();
-            removeInstruction(set);
-
-            // TODO When modification support is added to LogicList, rewrite this to modify instructions there
-            List<LogicInstruction> newInstructions = newBody.stream()
-                    .map(ix -> replaceAllArgs(ix, variable, result))
-                    .toList();
-
-            return buildLogicList(newBody.getAstContext(), newInstructions);
-        } else {
-            return newBody;
-        }
-    }
-
     private class InlineFunctionAction extends AbstractOptimizationAction {
         public InlineFunctionAction(AstContext astContext, int cost, double benefit) {
             super(astContext, cost, benefit);
@@ -217,7 +196,7 @@ private OptimizationResult inlineFunctionCall(AstContext call, int costLimit) {
         AstContext newContext = call.parent().createSubcontext(INLINE_CALL, 1.0);
         int insertionPoint = firstInstructionIndex(call);
         LogicList newBody = body.duplicateToContext(newContext);
-        insertInstructions(insertionPoint, swapReturnVariable(call, newBody));
+        insertInstructions(insertionPoint, newBody);
         // Remove original call instructions
         removeMatchingInstructions(ix -> ix.belongsTo(call));
 

mindcode/src/main/java/info/teksol/mindcode/compiler/optimization/LoopHoisting.java

@@ -8,7 +8,6 @@
 import info.teksol.mindcode.compiler.instructions.LogicInstruction;
 import info.teksol.mindcode.compiler.instructions.NoOpInstruction;
 import info.teksol.mindcode.compiler.optimization.OptimizationContext.LogicList;
-import info.teksol.mindcode.logic.ArgumentType;
 import info.teksol.mindcode.logic.LogicArgument;
 import info.teksol.mindcode.logic.LogicVariable;
 
@@ -176,13 +175,6 @@ private void addDependencies(AstContext loop, AstContext inspectedContext,
                     .filter(LogicVariable.class::isInstance)
                     .map(LogicVariable.class::cast)
                     .forEach(arg -> dependencies.computeIfAbsent(arg, a -> new HashSet<>()).addAll(inputs));
-
-            // Function return variables outside their functions are not loop invariant
-            instruction.inputArgumentsStream()
-                    .filter(LogicVariable.class::isInstance)
-                    .map(LogicVariable.class::cast)
-                    .filter(l -> l.getType() == ArgumentType.FUNCTION_RETVAL && !Objects.equals(l.getFunctionPrefix(), prefix))
-                    .forEachOrdered(arg -> dependencies.computeIfAbsent(arg, a -> new HashSet<>()).add(arg));
         } else {
             // This instruction isn't loop independent: it's unsafe, nondeterministic or nonlinear.
             // Add output variables as depending on themselves, removing their invariant status

mindcode/src/main/java/info/teksol/mindcode/logic/MindustryOpcodeVariants.java

@@ -263,8 +263,8 @@ private List<OpcodeVariant> initialize() {
         add(list, V6, V7A, S, NONE, Opcode.GOTOLABEL,   label("address"), label("marker"));
         add(list, V6, V7A, S, NONE, Opcode.PUSH,        block("memory"), in("value"));
         add(list, V6, V7A, S, NONE, Opcode.POP,         block("memory"), out("value"));
-        add(list, V6, V7A, S, NONE, Opcode.CALL,        label("callAddr"));
-        add(list, V6, V7A, S, NONE, Opcode.CALLREC,     block("memory"), label("callAddr"), label("retAddr"));
+        add(list, V6, V7A, S, NONE, Opcode.CALL,        label("callAddr"), out("retval"));
+        add(list, V6, V7A, S, NONE, Opcode.CALLREC,     block("memory"), label("callAddr"), label("retAddr"), out("retval"));
         add(list, V6, V7A, S, NONE, Opcode.RETURN,      block("memory"));
         add(list, V6, V7A, S, NONE, Opcode.GOTO,        in("address"), label("marker"));
         add(list, V6, V7A, S, NONE, Opcode.GOTOOFFSET,  label("address"), in("value"), in("offset"), label("marker"));

mindcode/src/test/java/info/teksol/mindcode/compiler/LogicInstructionLabelResolverTest.java

@@ -50,7 +50,7 @@ void resolvesVirtualInstructions() {
                                 createInstruction(JUMP, label0, Condition.ALWAYS),
                                 createInstruction(PUSH, cell1, a),
                                 createInstruction(POP, cell1, a),
-                                createInstruction(CALLREC, cell1, label1, label2),
+                                createInstruction(CALLREC, cell1, label1, label2, fn0retval),
                                 createInstruction(LABEL, label1),
                                 createInstruction(RETURN, cell1),
                                 createInstruction(LABEL, label2),