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

API: Add clear function to bitwise #50

Closed
wants to merge 2 commits into from
Closed

API: Add clear function to bitwise #50

wants to merge 2 commits into from

Conversation

FedericoBruzzone
Copy link
Contributor

Closes #49

These function complete the bitwise file.

  • The first one allow us to clean (set to 0) all bits from msg to idx;
  • The second one allow us to clean (set to 0) all bits from idx to 0

Signed-off-by: Federico Bruzzone [email protected]

@FedericoBruzzone
Copy link
Contributor Author

Some function (in particular set functions) on bitwise file uses in its body for example let mask = 0b1 << bit_idx and then *self &= mask.
Can I change it in *self &= (0b1 << bit_idx)?

guerinoni
guerinoni reviewed Sep 15, 2022
View reviewed changes
src/bitwise.rs Outdated Show resolved Hide resolved
src/bitwise.rs Outdated Show resolved Hide resolved
@FedericoBruzzone FedericoBruzzone force-pushed the clear-function branch 3 times, most recently from 57d1c49 to 4d188de Compare September 16, 2022 08:46
@FedericoBruzzone FedericoBruzzone force-pushed the clear-function branch 2 times, most recently from 1edf49a to 81c0e4c Compare September 23, 2022 23:17
gallottino
gallottino reviewed Sep 29, 2022
View reviewed changes
Copy link
Collaborator

@gallottino gallottino left a comment

Choose a reason for hiding this comment

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

I prefer to have only one method with left_index and right_index.

@FedericoBruzzone
Copy link
Contributor Author

Perfect, I'll merge my two functions 👍

@FedericoBruzzone FedericoBruzzone force-pushed the clear-function branch 2 times, most recently from 0dd8200 to d2c73f9 Compare September 29, 2022 12:11
gallottino
gallottino previously approved these changes Sep 29, 2022
View reviewed changes
@gallottino gallottino dismissed their stale review September 29, 2022 15:16

I missclicked

@gallottino
Copy link
Collaborator

Perfect, I'll merge my two functions 👍

I'm not sure that in this way there is an improvment. I don't like that someone has forced to start from msb or lsb.
For example:

101101

I want to clear only the two centered '1'. With this method I can't. Am I missing something?

@guerinoni @FedericoBruzzone
ARM: Add memory field into arm implementation of CPU
0ad8d8f 
This allows to fix `SingleDataTransfer::Ldr` istruction.

Signed-off-by: Federico Guerinoni <[email protected]>
@FedericoBruzzone
Copy link
Contributor Author

I did not understand.

I fixed everything!

@gallottino gallottino self-requested a review October 7, 2022 09:14
gallottino
gallottino requested changes Oct 7, 2022
View reviewed changes
Copy link
Collaborator

@gallottino gallottino left a comment

Choose a reason for hiding this comment

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

@FedericoBruzzone There is some problem in the history: you add a void merge from main to branch and your changes are not isolated from the rest. I think that for this pull request just a commit with "add clear_bits method in bitwise" must to appear.

Anyway we don't need this method yet, so I don't know if it is worth working on it.

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

@guerinoni guerinoni guerinoni left review comments

@gallottino gallottino gallottino requested changes

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

Successfully merging this pull request may close these issues.

Add clear function to bitwise.rs
3 participants
@FedericoBruzzone @gallottino @guerinoni