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.

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

Adjust operand order for vmerge and vcompress #185

Merged
merged 3 commits into from
Jan 13, 2023
Merged

Adjust operand order for vmerge and vcompress #185

merged 3 commits into from
Jan 13, 2023

Conversation

eopXD
Copy link
Collaborator

@eopXD eopXD commented Jan 11, 2023

This makes instructions of mnemonic vvm, vxm have consistent intrinsic
interface.

Before:

vint32m1_t vmerge_vvm_i32m1
  (vbool32_t mask, vint32m1_t op1, vint32m1_t op2, size_t vl);

After:

vint32m1_t vmerge_vvm_i32m1
  (vint32m1_t op1, vint32m1_t op2, vbool32_t selector, size_t vl);

Before:

vint32m1_t vcompress_vm_i32m1
  (vbool32_t mask, vint32m1_t src, size_t vl);

After:

vint32m1_t vcompress_vm_i32m1
  (vint32m1_t src, vbool32_t selector, size_t vl);

Resolves #140
Resolves #167

Corresponding changes in LLVM is posted here:

Copy link
Collaborator

@rofirrim rofirrim left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me.

I reckon for vmerge we followed the conditional operator (?) of C but I guess it becomes irregular with the rest of the intrinsics which adhere closely to the instruction operand order.

vcompress was likely a bit of an oversight so this one is not very controversial to me.

This makes instructions of mnemonic vvm, vxm have consistent intrinsic
interface.

Old:
vint32m1_t vmerge_vvm_i32m1
  (vbool32_t mask, vint32m1_t op1, vint32m1_t op2, size_t vl);
New:
vint32m1_t vmerge_vvm_i32m1
  (vint32m1_t op1, vint32m1_t op2, vbool32_t selector, size_t vl);

Old:
vint32m1_t vcompress_vm_i32m1
  (vbool32_t mask, vint32m1_t src, size_t vl);
New:
vint32m1_t vcompress_vm_i32m1
  (vint32m1_t src, vbool32_t selector, size_t vl);

Address following issues:
#140
#167

Signed-off-by: eop Chen <eop.chen@sifive.com>
@eopXD
Copy link
Collaborator Author

eopXD commented Jan 13, 2023

Change: Rebase to latest master.

@eopXD eopXD merged commit a6ac6a3 into riscv-non-isa:master Jan 13, 2023
@eopXD eopXD deleted the adjust-vmerge-vcompress-order branch January 13, 2023 01:28
eopXD added a commit to llvm/llvm-project that referenced this pull request Jan 13, 2023
From:
  vint32m1_t vmerge_vvm_i32m1 (vbool32_t mask, vint32m1_t op1, vint32m1_t op2, size_t vl);
  vint32m1_t vcompress_vm_i32m1 (vbool32_t mask, vint32m1_t src, size_t vl);

To:
  vint32m1_t vmerge_vvm_i32m1 (vint32m1_t op1, vint32m1_t op2, vbool32_t selector, size_t vl);
  vint32m1_t vcompress_vm_i32m1 (vint32m1_t src, vbool32_t selector, size_t vl);

Address issues:
riscv-non-isa/rvv-intrinsic-doc#140
riscv-non-isa/rvv-intrinsic-doc#167

Pull request:
riscv-non-isa/rvv-intrinsic-doc#185

Reviewed By: craig.topper

Differential Revision: https://reviews.llvm.org/D140686
CarlosAlbertoEnciso pushed a commit to SNSystems/llvm-debuginfo-analyzer that referenced this pull request Jan 13, 2023
From:
  vint32m1_t vmerge_vvm_i32m1 (vbool32_t mask, vint32m1_t op1, vint32m1_t op2, size_t vl);
  vint32m1_t vcompress_vm_i32m1 (vbool32_t mask, vint32m1_t src, size_t vl);

To:
  vint32m1_t vmerge_vvm_i32m1 (vint32m1_t op1, vint32m1_t op2, vbool32_t selector, size_t vl);
  vint32m1_t vcompress_vm_i32m1 (vint32m1_t src, vbool32_t selector, size_t vl);

Address issues:
riscv-non-isa/rvv-intrinsic-doc#140
riscv-non-isa/rvv-intrinsic-doc#167

Pull request:
riscv-non-isa/rvv-intrinsic-doc#185

Reviewed By: craig.topper

Differential Revision: https://reviews.llvm.org/D140686
veselypeta pushed a commit to veselypeta/cherillvm that referenced this pull request Jun 12, 2024
From:
  vint32m1_t vmerge_vvm_i32m1 (vbool32_t mask, vint32m1_t op1, vint32m1_t op2, size_t vl);
  vint32m1_t vcompress_vm_i32m1 (vbool32_t mask, vint32m1_t src, size_t vl);

To:
  vint32m1_t vmerge_vvm_i32m1 (vint32m1_t op1, vint32m1_t op2, vbool32_t selector, size_t vl);
  vint32m1_t vcompress_vm_i32m1 (vint32m1_t src, vbool32_t selector, size_t vl);

Address issues:
riscv-non-isa/rvv-intrinsic-doc#140
riscv-non-isa/rvv-intrinsic-doc#167

Pull request:
riscv-non-isa/rvv-intrinsic-doc#185

Reviewed By: craig.topper

Differential Revision: https://reviews.llvm.org/D140686
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants