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

Zvfh/Zvfhmin ACT #29

Open
2 of 13 tasks
jjscheel opened this issue Apr 11, 2023 · 19 comments
Open
2 of 13 tasks

Zvfh/Zvfhmin ACT #29

jjscheel opened this issue Apr 11, 2023 · 19 comments
Assignees
@jjscheel @allenjbaum

Comments

@jjscheel
Copy link
Contributor

jjscheel commented Apr 11, 2023 •
edited
Loading

Technical Group

Unprivileged Spec IC

ratification-pkg

Zvfh/Zvfhmin

Technical Liaison

Andrew Waterman

Task Category

Arch Tests

Task Sub Category

  • gcc
  • binutils
  • gdb
  • intrinsics
  • Java
  • KVM
  • ld
  • llvm
  • Linux kernel
  • QEMU
  • Spike

Ratification Target

2Q2023

Statement of Work (SOW)

Component names:
Zvfh/Zvfhmin

Requirements:
Extend the base vector ACT tests and test generation tooling to include Zvfhmin and Zvfh extensions for any VLEN less than 4k.

If necessary, the base vector testcase framework will be refactored as a part of this work so that the future vector "functions" can be supported more simply. This common vector framework should:

  • support base vector registers
  • support immediates
  • support vector configuration (SEW, LMUL, AVL)
  • support invalid configurations (e.g. LMUL*VLEN < EGW)
  • support different VLEN values

Deliverables:

  1. Assembly language tests
  2. Coverage models

Acceptance Criteria:

  1. Assembly language tests that meet the TestFormatSpec
  2. Coverage models using riscv-ctg YAML formatted schema Acceptance criteria
  3. Tests pass using the riscof framework

Projected timeframe:
6 months

SOW Signoffs:

  • Task group liaison sign-off
  • Development partner sign-off
  • ACT SIG sign-off date

Waiver

  • Freeze
  • Ratification

Pull Request Details

No response
@jjscheel jjscheel self-assigned this Apr 11, 2023
@jjscheel jjscheel moved this from As-planned to Blocked in RISC-V DevPartner Work Apr 11, 2023
@jjscheel
Copy link
Contributor Author

Need to find someone to help me complete the SOW content.

@allenjbaum
Copy link

allenjbaum commented Apr 11, 2023 via email

@jjscheel
Copy link
Contributor Author

@allenjbaum, while I agree on the vision, I want this issue in DevPartners to be simply Zvfh and Zvfhmin support. Then, we can utilize the experience to solve this bigger picture as it's own effort, in a separate set of SOWs. Is that ok?

@allenjbaum
Copy link

allenjbaum commented Apr 12, 2023 via email

@cmuellner
Copy link

cmuellner commented Apr 13, 2023 •
edited
Loading

I noticed that Christoff has submitted coverage files for Zvk, which is interesting - I would like to dig into those and see how he's done it.

No magic there. In the absence of fundamental vector support, these files are pretty useless.

What's needed is fundamental vector support, which includes at least:

  • support of vector registers (ok, that's the part that my coverage files somehow address)
  • support of immediates
  • support for vector configuration (SEW, LMUL, AVL)
  • support for invalid configurations (e.g. LMUL*VLEN < EGW)
  • support for different VLEN

I don't think this should be provided as part of any Zv* support.
Instead a dedicated task/SoW for ACT Vector enablement would be more appropriate and transparent.

@allenjbaum
Copy link

allenjbaum commented Apr 13, 2023 via email

@cmuellner
Copy link

We can already see the following when looking at the Zvk* ACT PR:

  • The PR won't be merged without coverage information
  • Coverage information depends on vector support as the foundation
  • Holding a PR open will lead to bit-rot and cause duplicated work (the Zvk* PR introduces vector support but is targeted for Zvk*)
  • It is obvious that every new Zv* extension PR will conflict with existing Zv* PRs (they need to duplicate work for limited vector support for their purposes)
  • It is unlikely that people will rewrite their PRs in several months (from scratch) when vector support is finally available (and likely incompatible with the code in the PRs)

So as long as we don't find a way to merge the Zv* ACT PRs, it does not make sense to write them (major parts will be deleted later anyway because the tests should be generated, not written).

It is probably more time efficient to focus on vector support first (or at least a subset of the complete vector support) and add Zvfh/Zvfhmin on top (which should be nothing more than adding a few cgf files). Almost all instructions of Zfh are listed in the chapter "Vector Floating-Point Instructions" of the vector spec (for SP FP and DP FP).

So I believe the following would be a reasonable order of tasks to move forward:

  • support of vector registers (ok, that's the part that my coverage files somehow address)
  • support of immediates
  • support for vector configuration (SEW, LMUL, AVL) - vset(i)vl(i).v
  • support for invalid configurations (e.g. LMUL*VLEN < EGW)
  • support for different VLEN
  • support for exception handling incl vstart support
  • support for masking
  • at this point the infrastructure should be complete and stable enough to add support for most vector instructions
  • support for floating point
  • support for Zvfh*
  • support for arithmetic
  • support for load/store instructions
  • support for other parts (alignment, fix-point arith, reduction, permutation, Zvl*, Zve*, etc)

CC: @pawks, as he might have some opinion here as well.

@pawks
Copy link

pawks commented Apr 13, 2023 •
edited
Loading

Preface - My understanding of the vector spec is rudimentary, so some of my comments below may be incorrect.

As far as vector goes, there are 4 categories of support required for getting ACT up.

  1. Support for architectural states(specifically vtype csr)
  2. Support for the vector register file and splitting up the value in the registers into an array of elements (needed for coverage calculation)
  3. Writing/Generating Tests
  4. Privilege support(testing illegal combinations)

These changes span across 2 to 4 tools:

  • riscv-config: 1
  • riscv-isac: 1, 2 & 4(some additional work would be required to effectively express the coverpoints)
  • riscv-ctg(optional): 3
  • riscof: 4(Maybe)

(major parts will be deleted later anyway because the tests should be generated, not written)

This is not entirely true. The policy for ACT is that we don't worry about where the tests come from. The test generator is a means to an end and not the norm. ACT is more concerned about coverage measurement and expression. As long as the coverage is sufficient and measurable, the tests can be handwritten. This is a handwritten test which is trying to test the architectural hazards among various instructions as they may appear in the real world applications.

support of vector registers (ok, that's the part that my coverage files somehow address)

The cgf files in the PR only add the relevant nodes. But that is not sufficient for measuring coverage. Appropriate changes will also be required in riscv-isac(regfile support, support for parsing information from the logs etc).

(they need to duplicate work for limited vector support for their purposes)

Looks like each subextension implements a constrained variant of the function for vectorisation of register values. In which case, I definitely think that writing a general function once should be the priority i.e the first vector subextension to have coverage support should implement them generally.

I agree with the order of the proposed changes mostly. I would definitely de-prioritize support for invalid configurations and exceptional handling towards the end. As always, privilege support is really tricky in itself. It would be easier to do that once the unpriv instruction support is added.

@cmuellner
Copy link

Thanks for your input! I think we all agree that partial vector support would be the right thing to.
However, we are confronted with the fact that Zv* extension TGs want ACT support for their ratification asap.

One point, where I'd like to add some more details:

(major parts will be deleted later anyway because the tests should be generated, not written)

This is not entirely true. The policy for ACT is that we don't worry about where the tests come from. The test generator is a means to an end and not the norm. ACT is more concerned about coverage measurement and expression. As long as the coverage is sufficient and measurable, the tests can be handwritten. This is a handwritten test which is trying to test the architectural hazards among various instructions as they may appear in the real world applications.

For floating-point tests, there are these ibm_b* functions, which attempt something similar: avoid purely randomized data, but specify properties that the test cases must fulfill (e.g. NaNs, normal numbers, etc). So we have abstract test case descriptions that define constraints for the generated tests. I think this could (and should) be applied to sha256sum0-rwp1.S as well. So if we define a property that teaches the test case generator to generate appropriate tests, then this property can be reused by other extensions/instructions.

So I believe that we should not have a single handwritten test case file. This would also reduce test case maintenance. For example, you don't need to adjust test cases manually if you change the test language (e.g. remove trapreg_sv and tramptbl_sv) because you can just regenerate all of them.

@allenjbaum
Copy link

allenjbaum commented Apr 16, 2023 via email

@cmuellner
Copy link

Those are internal to the trap handler, and they should not need to be changed for future tests. They are effectively spill/fill areas for trap handlers.

Why should a test case author ever have to care about such implementation details of the test suite? That is not even slightly related to most unpriv extensions. So my argument is, that a full-automation approach allows hiding such details and let the author focus on the actual test case.

@allenjbaum
Copy link

allenjbaum commented Apr 17, 2023 via email

@jjscheel
Copy link
Contributor Author

jjscheel commented May 4, 2023

@allenjbaum, can you review my proposed SOW in the first entry and let me know what changes you'd like to see?

Given that we're hoping that RIOS will pick this up after they complete the base Vector work, I don't have a problem building in the assumption that upon completion of this work, we have a base vector framework in ACT. How or when that work occurs (base vector SOW or this one) does not matter to me as long as it exists before we declare this one done.

@cmuellner, your thoughts are also welcome here as I used key points from your previous post.

THANKS!

@allenjbaum
Copy link

The only thing I would add is that coverage for Zvfh should include the coverage for Zfh operations (which would subsume Zfhmin). The interesting part of that is how it applies to the scaffolding part, .e.g, do you want coverage to be 100% for each lane individually, or is it adequate to do it collectively (under the assumption that each lane is identical so if you test one, you test them all). That is something that every vector test has to consider (i.e. it doesn't apply to all vector ops, specifically those that have inter-element operations like permutations, but that doesn't apply here)

@jjscheel
Copy link
Contributor Author

@allenjbaum, I understand your logic and generally agree going forward.

In this instance, however, isn't Prasanna already working on Zfh in issue #18? If true, this should make the Zvfh work simpler as the basic operation implementation should be done. Right?

@allenjbaum
Copy link

allenjbaum commented May 10, 2023 via email

@jjscheel
Copy link
Contributor Author

@allenjbaum, I understand your comments. However, I'm struggling to know what, if anything needs to change in the SOW. Can you explicitly state the text changes you'd like to see in the SOW description (first entry of this issue)?

Thanks!

@allenjbaum
Copy link

allenjbaum commented May 25, 2023 via email

@jjscheel
Copy link
Contributor Author

jjscheel commented Jun 8, 2023

Ok, I understand. I propose that we wait to decide whether we create a new SOW/item in the list until we have some more work done in the Arch Test SIG with the base Vector ACT. If you folks decide to start restructuring the initial vector ACT into framework and operations, then an additional SOW may not be needed.

@jjscheel jjscheel moved this from Blocked to As-planned in RISC-V DevPartner Work Feb 1, 2024
@jjscheel jjscheel moved this from As-planned to Blocked in RISC-V DevPartner Work Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jjscheel jjscheel

@allenjbaum allenjbaum

Labels
None yet
Projects
RISC-V DevPartner Work
Status: Blocked
Milestone
No milestone
Development

No branches or pull requests
4 participants
@cmuellner @jjscheel @pawks @allenjbaum