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 dirtyness propagation optimization to work fine with tweaks #2048

Merged
merged 1 commit into from
Sep 29, 2020

Conversation

brusherru
Copy link
Contributor

It fixes #2047

The bug was introduced in the 0.33.0 right in the commit 0880f41 😬

@brusherru brusherru added s:rt/c++ hotfix Issue/PR for a patch release labels Sep 28, 2020
@brusherru brusherru self-assigned this Sep 28, 2020
Copy link
Member

@nkrkv nkrkv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn’t it be more appropriate to move this business logic out of the template, to the helper definitions or even further? Templates are harder to maintain.

@brusherru brusherru force-pushed the fix-2047-tweak-outputs-dirtyness branch from ecb2e50 to 49a3e0d Compare September 28, 2020 15:02
@brusherru brusherru requested a review from nkrkv September 28, 2020 15:02
@evgenykochetkov
Copy link
Contributor

evgenykochetkov commented Sep 28, 2020

If I understand #2047 correctly, the following is happening:

digital-write node that wants to update only when UPD pin is touched, as stated by

#pragma XOD evaluate_on_pin disable
#pragma XOD evaluate_on_pin enable input_UPD

but the code generated in handleDebugProtocolMessages

g_transaction.node_{{ id }}_isNodeDirty = true;
g_transaction.node_{{ id }}_isOutputDirty_OUT = true;

(and maybe in some other place?) does not respect that.

I think that forcing generation of node_NNN_isOutputDirty_XXX and marking nodes as dirty in such situation is not right. The role of tweak node connected to pin with evaluate_on_pin disabled should be reduced to just storing the received value, and marking the pin and node dirty should be skipped.

@brusherru brusherru force-pushed the fix-2047-tweak-outputs-dirtyness branch from 49a3e0d to 9fc2298 Compare September 28, 2020 17:43
@brusherru
Copy link
Contributor Author

@nkrkv @evgenykochetkov check it out again, please :)

Copy link
Contributor

@evgenykochetkov evgenykochetkov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM👍🏻

@brusherru brusherru force-pushed the fix-2047-tweak-outputs-dirtyness branch from 9fc2298 to 7f279f5 Compare September 29, 2020 13:36
Copy link
Member

@nkrkv nkrkv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@brusherru brusherru merged commit f913fcf into 0.35.x Sep 29, 2020
@brusherru brusherru deleted the fix-2047-tweak-outputs-dirtyness branch September 29, 2020 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hotfix Issue/PR for a patch release s:rt/c++
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants