Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix action is_present field not being reset #2409

Merged
merged 29 commits into from
Sep 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
86e5bb7
For actions, add the status field to the is_present_fields array
erlingrj Sep 24, 2024
cf1ecac
Fix code-generator issues with is_present_fields and multiport and banks
erlingrj Sep 24, 2024
4e47ce9
Fix minor typos
erlingrj Sep 24, 2024
e01c51d
Try getting the action is_present field setup correct
erlingrj Sep 25, 2024
9104111
Bump reactor-c
erlingrj Sep 25, 2024
5a2f8cd
Typo
erlingrj Sep 25, 2024
08762b6
Trial and error...
erlingrj Sep 25, 2024
8d29e6b
Spotless
erlingrj Sep 25, 2024
be745a4
More trials
erlingrj Sep 25, 2024
6a9be53
Rework the assignment add some smoke tests
erlingrj Sep 25, 2024
70e2de7
Typo
erlingrj Sep 25, 2024
464e680
Spotless
erlingrj Sep 26, 2024
5017c83
Use runtime index of parent
erlingrj Sep 26, 2024
594cb13
REmove unused count variable
erlingrj Sep 26, 2024
4cfd1ce
Remove count incremet
erlingrj Sep 26, 2024
384a68a
Spotless
erlingrj Sep 27, 2024
4bf08db
Merge branch 'master' into fix-action-present-bug
edwardalee Sep 28, 2024
3b14a06
Typo
edwardalee Sep 28, 2024
439aeda
Simplified one iteration, elaborated test, and aligned reactor-c
edwardalee Sep 28, 2024
c626bb1
Extra cautious with branch that should not occur
edwardalee Sep 28, 2024
8331d1c
Small fix for branch that should not occur
edwardalee Sep 28, 2024
7f246be
Corrected order of endScopedBlock calls
edwardalee Sep 28, 2024
cdcfb33
Removed unnecessary scoped block
edwardalee Sep 28, 2024
d8a3f27
Corrected stride on action and run spotless
edwardalee Sep 28, 2024
db3ef25
Restored stride as needed
edwardalee Sep 28, 2024
232534f
Reversed simplification which didn't work
edwardalee Sep 28, 2024
fada318
Corrected simplication
edwardalee Sep 28, 2024
359aae0
Another refinement
edwardalee Sep 28, 2024
1cf9303
Sigh. Spotless
edwardalee Sep 28, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 68 additions & 30 deletions core/src/main/java/org/lflang/generator/c/CGenerator.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand All @@ -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;");
Expand All @@ -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");
}
Expand All @@ -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.
Expand All @@ -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();
Expand All @@ -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;");

Expand All @@ -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());
Expand Down Expand Up @@ -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();
Expand Down
12 changes: 6 additions & 6 deletions core/src/main/java/org/lflang/generator/c/CUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
*/
Expand Down
30 changes: 30 additions & 0 deletions test/C/src/ActionIsPresentReset.lf
Original file line number Diff line number Diff line change
@@ -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");
}
=}
}
71 changes: 71 additions & 0 deletions test/C/src/multiport/BankMultiportIsPresentSetup.lf
Original file line number Diff line number Diff line change
@@ -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
}
26 changes: 26 additions & 0 deletions test/C/src/multiport/BankOfActionsIsPresentSetup.lf
Original file line number Diff line number Diff line change
@@ -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
}
Loading