Conversation
Greptile SummaryThis PR fixes
Confidence Score: 4/5Safe to merge for the primary fix; the change correctly recovers user-set pin activities that propagation cannot forward. The two-line change is minimal and well-targeted: it fixes the described case where flop and macro output pins silently reported zero activity despite having a user annotation. One related gap remains — the The Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[findActivity called for pin] --> B{Pin is constant?}
B -- Yes --> C[Return 0.0/0.0 constant]
B -- No --> D{Pin is clock?}
D -- Yes --> E{activity_map_ has non-unknown entry?}
E -- Yes --> F[Return activity_map_ entry]
E -- No --> G[Return clock-computed activity]
D -- No --> H{global_activity_ is set?}
H -- Yes --> I[Return global_activity_ - user_activity_map_ never reached]
H -- No --> J{activity_map_ has non-unknown entry?}
J -- Yes --> K[Return activity_map_ entry]
J -- No --> L{user_activity_map_ has entry? NEW fallback added by this PR}
L -- Yes --> M[Return user_activity_map_ entry]
L -- No --> N[Return 0.0/0.0 unknown]
|
| if (user_activity_map_.hasKey(pin)) | ||
| return user_activity_map_[pin]; |
There was a problem hiding this comment.
Missing comment for new fallback logic
Per the team's convention of commenting complex code blocks, this new fallback should have a brief comment explaining why user_activity_map_ is consulted here — specifically that some pins (flop outputs, macro outputs) cannot be propagated through, so their user-set activities never reach activity_map_, making this check the last resort before returning unknown. Without the comment, it's non-obvious why this check appears after the activity_map_ lookup rather than before it.
Rule Used: Always add comments to explain the purpose of comp... (source)
Learned From
Silimate/preqorsor-vscode#33
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Pins specified by the user using set_power_activity are not queried in findActivity because they appear in user_activity_map. This is not good because some pins can't be propagated through in some cases (flops or macro outputs)
This leads to pins being ignored and reported as having 0 activity/duty when they were actually present as they're set by the user. Before we say an activity origin is unknown, query this map and return the value