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

Replace vmandnot and vmornot with vmandn and vmorn - Issue #421 #465

Merged
merged 4 commits into from
May 9, 2024
Merged

Replace vmandnot and vmornot with vmandn and vmorn - Issue #421 #465

merged 4 commits into from
May 9, 2024

Conversation

ShivangMishra
Copy link
Contributor

No description provided.

@ShivangMishra ShivangMishra mentioned this pull request May 8, 2024
Closed
Alasdair
Alasdair reviewed May 8, 2024
View reviewed changes
Copy link
Collaborator

@Alasdair Alasdair left a comment

Choose a reason for hiding this comment

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

Wondering if we should also change MM_VMANDNOT to MM_VMANDN? Otherwise lgtm.

@ShivangMishra
Copy link
Contributor Author

Wondering if we should also change MM_VMANDNOT to MM_VMANDN? Otherwise lgtm.

Wondering if we should also change MM_VMANDNOT to MM_VMANDN? Otherwise lgtm.

so...should I change it?

@Alasdair
Copy link
Collaborator

Alasdair commented May 8, 2024

I think it would be better if we updated both in a single PR, if you would be willing to make that change also.

@ShivangMishra
Copy link
Contributor Author

looks ok now?

Alasdair
Alasdair reviewed May 8, 2024
View reviewed changes
Copy link
Collaborator

@Alasdair Alasdair left a comment

Choose a reason for hiding this comment

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

I think the indention needs adjustment, but otherwise should be fine.

model/riscv_insts_vext_mask.sail Outdated Show resolved Hide resolved
@github-actions GitHub Actions
Copy link

github-actions bot commented May 8, 2024 •
edited
Loading

Test Results

712 tests  ±0   712 ✅ ±0   0s ⏱️ ±0s
  6 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit 5b7df52. ± Comparison against base commit f65c4b2.

♻️ This comment has been updated with latest results.

@ShivangMishra
Copy link
Contributor Author

@Timmmm @Alasdair can this be merged now?

Timmmm
Timmmm approved these changes May 8, 2024
View reviewed changes
Copy link
Collaborator

@Timmmm Timmmm left a comment

Choose a reason for hiding this comment

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

Yeah LGTM. Can you squash your commits?

@Alasdair Alasdair merged commit e3c5b0a into riscv:master May 9, 2024
2 checks passed
@Alasdair
Copy link
Collaborator

Alasdair commented May 9, 2024

You can squash the commits when doing a merge as a maintainer, and it lets you update the message, so I just went ahead and did it.

Thanks for the fix @ShivangMishra

KotorinMinami pushed a commit to KotorinMinami/sail-riscv that referenced this pull request May 9, 2024
@ShivangMishra @KotorinMinami
Replace vmandnot and vmornot with vmandn and vmorn - Issue riscv#421 (r…
4a5cdde 
…iscv#465)

Updates the instruction mnemonics for vmandn and vmorn

* replace mnemonics "vmandnot" and "vmornot" with "vmandn" and "vmorn" respectively

* renamed MM_VMORNOT and MM_VMANDNOT to match mnemonics
bacam pushed a commit to bacam/sail-riscv that referenced this pull request Jun 4, 2024
@ShivangMishra @bacam
Replace vmandnot and vmornot with vmandn and vmorn - Issue riscv#421 (r…
536ad6b 
…iscv#465)

Updates the instruction mnemonics for vmandn and vmorn

* replace mnemonics "vmandnot" and "vmornot" with "vmandn" and "vmorn" respectively

* renamed MM_VMORNOT and MM_VMANDNOT to match mnemonics
ThinkOpenly pushed a commit to ThinkOpenly/sail-riscv that referenced this pull request Jul 3, 2024
@ShivangMishra @ThinkOpenly
Replace vmandnot and vmornot with vmandn and vmorn - Issue riscv#421 (r…
c62ab4d 
…iscv#465)

Updates the instruction mnemonics for vmandn and vmorn

* replace mnemonics "vmandnot" and "vmornot" with "vmandn" and "vmorn" respectively

* renamed MM_VMORNOT and MM_VMANDNOT to match mnemonics
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Alasdair Alasdair Alasdair approved these changes

@Timmmm Timmmm Timmmm 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.
3 participants
@ShivangMishra @Alasdair @Timmmm