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.

Sign up for GitHub

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

Jump to bottom

✂️ Changes for #1 #15

Merged
merged 2 commits into from
Mar 6, 2018
Merged

✂️ Changes for #1 #15

merged 2 commits into from
Mar 6, 2018

Conversation

raulbehl
Copy link
Contributor

@raulbehl raulbehl commented Mar 5, 2018

  • Moved the branch resolve engine to ALU
  • Updated conditions in the ALU code to account for different
    type of branches as well
  • Update the port definitions wherever required

@raulbehl
✂️ Changes for openhwgroup#1
8f2c2a0 
  - Moved the branch resolve engine to ALU
  - Updated conditions in the ALU code to account for different
    type of branches as well
  - Update the port definitions wherever required
@raulbehl raulbehl mentioned this pull request Mar 5, 2018
Closed
@zarubaf
Copy link
Contributor

zarubaf commented Mar 6, 2018 •
edited

That looks perfect to me! If you could just submit the small patch to the above review I am more than happy to merge.

Thansk a lot for your contribution! 😃

@raulbehl
Copy link
Contributor Author

raulbehl commented Mar 6, 2018

Hi @zarubaf,
Thank you for reviewing the branch. Sorry but I have a query regarding your comment. Do you want just a diff patch or there are any further changes also required? As I don't see any comments on the files changed section, so I am guessing the diff patch should be it?
Thank you :)

zarubaf
zarubaf requested changes Mar 6, 2018
View reviewed changes
src/alu.sv Outdated
if (operator_i == SLTS)
if ((operator_i == SLTS) |
(operator_i == LTS) |
(operator_i == GES))
Copy link
Contributor

Choose a reason for hiding this comment

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

One small note here: Can we change the operator to he boolean operator e.g.: ||? I know I have not been consistent myself but lets try not to introduce any further inconsitencies ;-)

@zarubaf
Copy link
Contributor

zarubaf commented Mar 6, 2018 •
edited

@raulbehl Sorry I have not pressed the submit button :-) I think you can see the request for change now.

@raulbehl
Update alu.sv based on review comments
50506f8 
Updated the if condition to use "||" instead of "|" operator
@raulbehl
Copy link
Contributor Author

raulbehl commented Mar 6, 2018

Sure. I have made the updates now.

@zarubaf
Copy link
Contributor

zarubaf commented Mar 6, 2018

Thanks!

@zarubaf zarubaf merged commit c74bb21 into openhwgroup:master Mar 6, 2018
@raulbehl raulbehl deleted the alu_brn_resolve branch March 6, 2018 16:51
OttG pushed a commit to OttG/cva6 that referenced this pull request Jul 6, 2021
@zarubaf
Merge pull request openhwgroup#15 from raulbehl/alu_brn_resolve
e3ee2b5 
✂️ Changes for #1
alaaibrah97 pushed a commit to alaaibrah97/cva6 that referenced this pull request Mar 29, 2022
@Jbalkind
Merge pull request openhwgroup#15 from msfschaffner/openpiton-dev
73ea17e 
Remove inferred latches, fixes to enable VCS for Ariane, add new PLIC and bump Ariane version.
luca-valente pushed a commit to AlSaqr-platform/cva6 that referenced this pull request Jul 20, 2023
@niwis
cva6/icache_axi: Fix addr width (openhwgroup#15)
608fd45 
Signed-off-by: Nils Wistoff <nwistoff@iis.ee.ethz.ch>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zarubaf zarubaf zarubaf approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@raulbehl @zarubaf