Skip to content

Conversation

@hfthra
Copy link

@hfthra hfthra commented Jan 20, 2026

fixed issues in format, comments, code , documenatation related to coprocessor-generator

Copy link
Contributor

@Joonari Joonari left a comment

Choose a reason for hiding this comment

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

Some minor style issues.

oredsignalsR = oredsignals;
BitwiseOr bitwiseOred(bitwiseAnded, andedSignals);
andedSignals = bitwiseOred;
oredSignalsR = bitwiseOred;
Copy link
Contributor

Choose a reason for hiding this comment

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

What the R stand for in oredSignalsR? It should be expanded.

/**
* BEM constructor for the Coprocessor generation.
* Overloaded BEM constructor related to CV-X-If and ROCC coprocessor generation.
* RISCV instrucion encodings are created based on the coproInterF boolean.
Copy link
Contributor

Choose a reason for hiding this comment

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

The F in the boolean name mentioned here should also be expanded for consistent style.

@hfthra
Copy link
Author

hfthra commented Jan 20, 2026

fixed the style issues mentioned by @Joonari

@pjaaskel pjaaskel requested a review from Joonari January 20, 2026 14:37
@pjaaskel
Copy link
Contributor

Why doesn't it recognize the CI workflow? Is this rebased on the latest main?

@pjaaskel
Copy link
Contributor

Ah, this is a merge request to the other branch. Please just rebase on it and then rebase on main to submit a PR towards main.

@hfthra
Copy link
Author

hfthra commented Jan 20, 2026

Added updates in CHANGES and fixed the issues pointed by @pjaaskel

@pjaaskel
Copy link
Contributor

Please just rebase on the other branch and then rebase on main and submit a PR towards main instead to the other PR.

@pjaaskel
Copy link
Contributor

LGTM after you send a PR against the main branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants