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

vector: crypto: fix constraint checks for vector-crypto instructions #1888

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tsewei-lin
Copy link

These are the changes:

  • Zvkg (vghsh.vv, vgmul.vv)
    • vl must be a multiple of EGS=4. (spec p.13)
    • Check alignment of vd, vs1, vs2 with lmul
  • Zvksh (vsm3c.vi, vsm3me.vv)
    • vstart, vl must be multiple of EGS=4 (spec p.17)
    • Check alignment of vd, vs1, vs2 with lmul
  • Zvksed (vsm4k.[vi,vs,vv])
    • vstart, vl must be multiple of EGS=4 (spec p.16)
    • Check alignment of vd, vs1, vs2 with lmul
    • For vsm4r.vs, check overlap between vs2 and vd (spec p.7)
  • Zvbb (vwsll.[vv,vx,vi])
    • Check alignment of vd, vs1, vs2 with lmul (for widening instructions)
    • Check overlap between vs2 and vd
  • Zvkned
    • vstart, vl must be multiple of EGS=4 (spec p.14)
    • Check alignment of vd, vs1, vs2 with lmul
    • For vaes*.vs, check overlap between vs2 and vd (spec p.7)
  • Zvknh
    • Check alignment of vd, vs1, vs2 with lmul

These are the changes:
- Zvkg (vghsh.vv, vgmul.vv)
    - vl must be a multiple of EGS=4. (spec p.13)
    - Check alignment of vd, vs1, vs2 with lmul
- Zvksh (vsm3c.vi, vsm3me.vv)
    - vstart, vl must be multiple of EGS=4 (spec p.17)
    - Check alignment of vd, vs1, vs2 with lmul
- Zvksed (vsm4k.[vi,vs,vv])
    - vstart, vl must be multiple of EGS=4 (spec p.16)
    - Check alignment of vd, vs1, vs2 with lmul
    - For vsm4r.vs, check overlap between vs2 and vd (spec p.7)
- Zvbb (vwsll.[vv,vx,vi])
    - Check alignment of vd, vs1, vs2 with lmul	(for widening instructions)
    - Check overlap between vs2 and vd
- Zvkned
    - vstart, vl must be multiple of EGS=4 (spec p.14)
    - Check alignment of vd, vs1, vs2 with lmul
    - For vaes*.vs, check overlap between vs2 and vd (spec p.7)
- Zvknh
    - Check alignment of vd, vs1, vs2 with lmul
@tsewei-lin
Copy link
Author

@chihminchao Please have a look

@chihminchao
Copy link
Contributor

It is good to me and the change has passed internal test.

@nibrunieAtSi5 I think it covers part of your PR #1815

@aswaterman
Copy link
Collaborator

ping me when it's ready to merge.

Copy link
Contributor

@nibrunieAtSi5 nibrunieAtSi5 left a comment

Choose a reason for hiding this comment

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

If I am not mistaken this PR lacks the checks for EMUL for the scalar group operands in .vs operations (that I tried to put in https://github.com/riscv-software-src/riscv-isa-sim/pull/1815/files). I don't think there are covered by VI_CHECK_SSS

Copy link
Contributor

@nibrunieAtSi5 nibrunieAtSi5 left a comment

Choose a reason for hiding this comment

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

Moreover, using require_noover with 1 as size for the scalar element group insn.rs2() only works for EGW = VLEN, I think the value needs to be different when EGW > VLEN.

@tsewei-lin
Copy link
Author

@nibrunieAtSi5 should I adopt the macro you add for .vs instruction in your PR for EMUL and overlap checks? (require_no_overlap_eglmul and require_vs2_align_eglmul specifically)

@nibrunieAtSi5
Copy link
Contributor

@nibrunieAtSi5 should I adopt the macro you add for .vs instruction in your PR for EMUL and overlap checks? (require_no_overlap_eglmul and require_vs2_align_eglmul specifically)

My macros were only but one suggestion. If you think they make sense, feel free to adopt them.

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.

4 participants