Merge pull request #2409 from lf-lang/fix-action-present-bug

Fix action is_present field not being reset

core/src/main/java/org/lflang/generator/c/CGenerator.java

@@ -1375,10 +1375,6 @@ private void recordBuiltinTriggers(ReactorInstance instance) {
     }
   }
 
-  /**
-   * Generate code to set up the tables used in _lf_start_time_step to decrement reference counts
-   * and mark outputs absent between time steps. This function puts the code into startTimeStep.
-   */
   /**
    * Generate code to set up the tables used in _lf_start_time_step to decrement reference counts
    * and mark outputs absent between time steps. This function puts the code into startTimeStep.
@@ -1410,6 +1406,9 @@ private void generateStartTimeStep(ReactorInstance instance) {
 
           temp.pr("// Add port " + port.getFullName() + " to array of is_present fields.");
 
+          // We will be iterating over instance (if a bank) and the parent of the port (if a bank).
+          var width = instance.getWidth() * port.getParent().getWidth();
+
           if (!Objects.equal(port.getParent(), instance)) {
             // The port belongs to contained reactor, so we also have
             // iterate over the instance bank members.
@@ -1418,16 +1417,29 @@ private void generateStartTimeStep(ReactorInstance instance) {
             temp.startScopedBlock(instance);
             temp.startScopedBankChannelIteration(port, null);
           } else {
+            // This branch should not occur because if the port's parent is instance and the port
+            // is an effect of a reaction of instance, then the port must be an output, not an
+            // input.
+            // Nevertheless, leave this here in case we missed something.
             temp.startScopedBankChannelIteration(port, "count");
+            width = port.getParent().getWidth();
           }
           var portRef = CUtil.portRefNested(port);
           var con = (port.isMultiport()) ? "->" : ".";
 
+          var indexString =
+              String.valueOf(enclaveInfo.numIsPresentFields)
+                  + " + ("
+                  + CUtil.runtimeIndex(instance.getParent())
+                  + ") * "
+                  + width * port.getWidth()
+                  + " + count";
+
           temp.pr(
               enclaveStruct
                   + ".is_present_fields["
-                  + enclaveInfo.numIsPresentFields
-                  + " + count] = &"
+                  + indexString
+                  + "] = &"
                   + portRef
                   + con
                   + "is_present;");
@@ -1436,19 +1448,19 @@ private void generateStartTimeStep(ReactorInstance instance) {
               CExtensionUtils.surroundWithIfFederatedDecentralized(
                   enclaveStruct
                       + "._lf_intended_tag_fields["
-                      + enclaveInfo.numIsPresentFields
-                      + " + count] = &"
+                      + indexString
+                      + "] = &"
                       + portRef
                       + con
                       + "intended_tag;"));
 
-          enclaveInfo.numIsPresentFields += port.getWidth() * port.getParent().getTotalWidth();
+          enclaveInfo.numIsPresentFields += port.getParent().getTotalWidth() * port.getWidth();
 
           if (!Objects.equal(port.getParent(), instance)) {
             temp.pr("count++;");
+            temp.endScopedBankChannelIteration(port, null);
             temp.endScopedBlock();
             temp.endScopedBlock();
-            temp.endScopedBankChannelIteration(port, null);
           } else {
             temp.endScopedBankChannelIteration(port, "count");
           }
@@ -1461,18 +1473,29 @@ private void generateStartTimeStep(ReactorInstance instance) {
 
     for (ActionInstance action : instance.actions) {
       foundOne = true;
-      temp.startScopedBlock(instance);
+
+      // Build the index into `is_present_fields` for this action.
+      var indexString =
+          String.valueOf(enclaveInfo.numIsPresentFields)
+              + " + ("
+              + CUtil.runtimeIndex(instance.getParent())
+              + ") * "
+              + action.getParent().getWidth();
+
+      if (instance.isBank()) {
+        indexString += " +  " + CUtil.bankIndexName(instance);
+      }
 
       temp.pr(
           String.join(
               "\n",
               "// Add action " + action.getFullName() + " to array of is_present fields.",
-              enclaveStruct + ".is_present_fields[" + enclaveInfo.numIsPresentFields + "] ",
-              "        = &"
+              enclaveStruct + ".is_present_fields[" + indexString + "]",
+              "        = (bool *) &"
                   + containerSelfStructName
-                  + "->_lf_"
+                  + "->_lf__"
                   + action.getName()
-                  + ".is_present;"));
+                  + ".status;"));
 
       // Intended_tag is only applicable to actions in federated execution with decentralized
       // coordination.
@@ -1481,18 +1504,14 @@ private void generateStartTimeStep(ReactorInstance instance) {
               String.join(
                   "\n",
                   "// Add action " + action.getFullName() + " to array of intended_tag fields.",
-                  enclaveStruct
-                      + "._lf_intended_tag_fields["
-                      + enclaveInfo.numIsPresentFields
-                      + "] ",
+                  enclaveStruct + "._lf_intended_tag_fields[" + indexString + "]",
                   "        = &"
                       + containerSelfStructName
                       + "->_lf_"
                       + action.getName()
                       + ".intended_tag;")));
 
       enclaveInfo.numIsPresentFields += action.getParent().getTotalWidth();
-      temp.endScopedBlock();
     }
     if (foundOne) startTimeStep.pr(temp.toString());
     temp = new CodeBuilder();
@@ -1506,17 +1525,35 @@ private void generateStartTimeStep(ReactorInstance instance) {
         temp.pr("int count = 0; SUPPRESS_UNUSED_WARNING(count);");
         temp.startScopedBlock(child);
 
-        var channelCount = 0;
+        // Need to find the total number of channels over all output ports of the child before
+        // generating the
+        // iteration.
+        var totalChannelCount = 0;
+        for (PortInstance output : child.outputs) {
+          if (!output.getDependsOnReactions().isEmpty()) {
+            totalChannelCount += output.getWidth();
+          }
+        }
         for (PortInstance output : child.outputs) {
           if (!output.getDependsOnReactions().isEmpty()) {
             foundOne = true;
-            temp.pr("// Add port " + output.getFullName() + " to array of is_present fields.");
+            temp.pr(
+                "// Add output port " + output.getFullName() + " to array of is_present fields.");
             temp.startChannelIteration(output);
+            var indexString =
+                "("
+                    + CUtil.runtimeIndex(instance)
+                    + ") * "
+                    + totalChannelCount * child.getWidth()
+                    + " + "
+                    + enclaveInfo.numIsPresentFields
+                    + " + count";
+
             temp.pr(
                 enclaveStruct
                     + ".is_present_fields["
-                    + enclaveInfo.numIsPresentFields
-                    + " + count] = &"
+                    + indexString
+                    + "] = &"
                     + CUtil.portRef(output)
                     + ".is_present;");
 
@@ -1526,22 +1563,23 @@ private void generateStartTimeStep(ReactorInstance instance) {
                 CExtensionUtils.surroundWithIfFederatedDecentralized(
                     String.join(
                         "\n",
-                        "// Add port " + output.getFullName() + " to array of intended_tag fields.",
+                        "// Add output port "
+                            + output.getFullName()
+                            + " to array of intended_tag fields.",
                         enclaveStruct
                             + "._lf_intended_tag_fields["
-                            + enclaveInfo.numIsPresentFields
-                            + " + count] = &"
+                            + indexString
+                            + "] = &"
                             + CUtil.portRef(output)
                             + ".intended_tag;")));
 
             temp.pr("count++;");
-            channelCount += output.getWidth();
             temp.endChannelIteration(output);
           }
         }
-        enclaveInfo.numIsPresentFields += channelCount * child.getTotalWidth();
         temp.endScopedBlock();
         temp.endScopedBlock();
+        enclaveInfo.numIsPresentFields += totalChannelCount * child.getTotalWidth();
       }
     }
     if (foundOne) startTimeStep.pr(temp.toString());
@@ -2027,7 +2065,7 @@ protected boolean targetLanguageIsCpp() {
    *     if there is none), and the message (or an empty string if there is none).
    */
   @Override
-  public GeneratorBase.ErrorFileAndLine parseCommandOutput(String line) {
+  public ErrorFileAndLine parseCommandOutput(String line) {
     var matcher = compileErrorPattern.matcher(line);
     if (matcher.find()) {
       var result = new ErrorFileAndLine();

core/src/main/java/org/lflang/generator/c/CUtil.java

@@ -93,9 +93,9 @@ public static String actionRef(ActionInstance instance, String runtimeIndex) {
   }
 
   /**
-   * Return a default name of a variable to refer to the bank index of a reactor in a bank. This is
-   * has the form uniqueID_i where uniqueID is an identifier for the instance that is guaranteed to
-   * be different from the ID of any other instance in the program. If the instance is not a bank,
+   * Return a default name of a variable to refer to the bank index of a reactor in a bank. This has
+   * the form uniqueID_i, where uniqueID is an identifier for the instance that is guaranteed to be
+   * different from the ID of any other instance in the program. If the instance is not a bank,
    * return "0".
    *
    * @param instance A reactor instance.
@@ -106,9 +106,9 @@ public static String bankIndex(ReactorInstance instance) {
   }
 
   /**
-   * Return a default name of a variable to refer to the bank index of a reactor in a bank. This is
-   * has the form uniqueID_i where uniqueID is an identifier for the instance that is guaranteed to
-   * be different from the ID of any other instance in the program.
+   * Return a default name of a variable to refer to the bank index of a reactor in a bank. This has
+   * the form uniqueID_i, where uniqueID is an identifier for the instance that is guaranteed to be
+   * different from the ID of any other instance in the program.
    *
    * @param instance A reactor instance.
    */

test/C/src/ActionIsPresentReset.lf

@@ -0,0 +1,30 @@
+target C {
+  timeout: 7 msecs,
+  fast: true
+}
+
+main reactor {
+  logical action a
+  logical action b
+
+  reaction(startup) -> a {=
+    lf_schedule(a, MSEC(1));
+  =}
+
+  reaction(a, b) -> a, b {=
+    if (a->is_present) {
+        printf("A");
+        lf_schedule(b, MSEC(2));
+    }
+    if (b->is_present) {
+        printf("B");
+        lf_schedule(a, MSEC(1));
+    }
+
+    lf_print(" at %d msecs with triggers (%d,%d)", lf_time_logical_elapsed() / MSEC(1), a->is_present, b->is_present);
+
+    if (a->is_present && b->is_present) {
+      lf_print_error_and_exit("Both triggers should not be present");
+    }
+  =}
+}

test/C/src/multiport/BankMultiportIsPresentSetup.lf

@@ -0,0 +1,71 @@
+// Smoke test for checking that the `is_present_fields` is setup correctly
+// for a complicated hierarchy of banks with multiports and actions.
+target C
+
+reactor R1 {
+  logical action a1
+  logical action a2
+  output[2] out: int
+
+  reaction(startup) -> out {=
+    lf_set(out[0], 42);
+  =}
+
+  reaction(startup) -> out {=
+    lf_set(out[1], 43);
+  =}
+}
+
+reactor R4 {
+  logical action a1
+  logical action a2
+  input[3] in: int
+  input[3] in2: int
+  input[2] in3: int
+
+  reaction(in) {=
+    lf_print("in = [%d, %d, %d]", in[0]->value, in[1]->value, in[2]->value);
+  =}
+
+  reaction(in2) {=
+    lf_print("in2 = [%d, %d, %d]", in2[0]->value, in2[1]->value, in2[2]->value);
+  =}
+
+  reaction(in3) {=
+    lf_print("in3 = [%d, %d]", in3[0]->value, in3[1]->value);
+  =}
+}
+
+reactor R2 {
+  logical action a1
+  logical action a2
+  r1 = new[3] R1()
+  r4 = new[2] R4()
+  r1.out -> r4.in
+
+  reaction(startup) -> r4.in2, r4.in3 {=
+    lf_set(r4[0].in2[0], 44);
+    lf_set(r4[1].in2[0], 45);
+    lf_set(r4[0].in2[1], 46);
+    lf_set(r4[1].in2[1], 47);
+    lf_set(r4[0].in2[2], 48);
+    lf_set(r4[1].in2[2], 49);
+
+    lf_set(r4[0].in3[0], 50);
+    lf_set(r4[1].in3[0], 51);
+    lf_set(r4[0].in3[1], 52);
+    lf_set(r4[1].in3[1], 53);
+  =}
+}
+
+reactor R3 {
+  r2 = new[4] R2()
+  logical action a1
+  logical action a2
+}
+
+main reactor {
+  r = new[2] R3()
+  logical action a1
+  logical action a2
+}

test/C/src/multiport/BankOfActionsIsPresentSetup.lf

@@ -0,0 +1,26 @@
+// Smoke test for checking that the `is_present_fields` is setup correctly
+// for a complicated hierarchy of nested banks with actions.
+target C
+
+reactor R1 {
+  logical action a1
+  logical action a2
+}
+
+reactor R2 {
+  r1 = new[3] R1()
+  logical action a1
+  logical action a2
+}
+
+reactor R3 {
+  r2 = new[4] R2()
+  logical action a1
+  logical action a2
+}
+
+main reactor {
+  r = new[2] R3()
+  logical action a1
+  logical action a2
+}