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

Extension subset problem encountered while reusing B ext #81

Closed
ksyx opened this issue Mar 11, 2021 · 6 comments
Closed

Extension subset problem encountered while reusing B ext #81

ksyx opened this issue Mar 11, 2021 · 6 comments
Labels
Toolchain Something todo with binutils / gcc / LLVM

Comments

@ksyx
Copy link
Contributor

ksyx commented Mar 11, 2021

We are currently implementing LLVM MC Layer support for K extension and while working on further separating the instructions into corresponding function sets, especially those reused from B extension, we found this relationship with B extension as follows. This indicates that K extension reuses part of but not all of instructions from Zbp and Zbc.
Instruction correspondence of reused instructions from B extension
While implementing MC Layer, we need to set a Predicate property for instructions to indicate the features required for this instructions to work, like HasStdExtZbp, IsRV64, etc. And now we have faced a problem that, if we need to only include those instructions used by K extension, and not use any extra instructions like shflw for a random example, we would need to modify the definition file (.td tablegen file) of B extension to further group those instructions, which is a bit invasive especially considering that the Zbc is marked with an asterisk, "means the extensions are expected to be unchanged in the official version". The other solution is to reuse the whole subset of Zbp and Zbc instructions, and this brings redundant instructions used.

Which of the solution above (modify B extension / reuse whole set of Zbp and Zbc) is better or any other ideas?

@ben-marshall
Copy link
Member

Hi @ksyx

So, my reading of this is that there is no actual problem with the groups, but implementing them in a way that LLVM understands is difficult. Is that right?

I'm not an LLVM expert, so I'd suggest you talk to the people who submitted the Bitmanip code to LLVM, or ask in the toolchains and runtimes group for the best way to manage this. I understand crypto is the first / only extension which shares instructions with another extension this way. It sounds like that's simply broken an implementation assumption for some people.

We cannot simply re-use the whole set of Zbp and Zbc instructions, since that would be incorrect with respect to the specification.

You could try creating some "pseudo-groups", and each "official" group then enables one or more "pseudo-groups"? For example, here in the GCC/binutils for Bitmanip, the rotate instructions can be enabled by either the B, Zbp and Zbb flags. Could a similar mechanism be used here?

I'm sorry I can't give you a direct answer, I hope that gives you some things to investigate? Let me know how you get on and I'll help out as best I can.

Thanks,
Ben

@ksyx
Copy link
Contributor Author

ksyx commented Mar 11, 2021

Thanks a lot for this reply! Yeah there is nothing wrong with the grouping but introduces some problems while implementing. The contexts and resources provided are helpful and I will dig into it.

@ben-marshall ben-marshall added the Toolchain Something todo with binutils / gcc / LLVM label Mar 11, 2021
@ksyx
Copy link
Contributor Author

ksyx commented Mar 12, 2021

Hi Ben,
After looking into this, I found implementing this in GCC/binutils may have the same problem as in LLVM. The link pointing to riscv-opc.c in the last comment uses definition INSN_CLASS_B_OR_ZBS, which is defined here. Thus if we need to group this ror instruction into Zkb we may need to rename all uses of INSN_CLASS_B_OR_ZBB_OR_ZBP into INSN_CLASS_B_OR_ZBB_OR_ZBP_ZKB, for example. So this might be a bit invasive toward the definition code of B extension. There is the same problem encountered when implementing in LLVM. Thus this might be a wider problem than in LLVM alone?
This is the progress I have made so far from my understanding and please correct me if I am wrong since I have not much experience in developing in GCC framework.

@ben-marshall
Copy link
Member

Hi @ksyx

I found implementing this in GCC/binutils may have the same problem as in LLVM.

Okay, well that's sort of good news in that maybe there's a solution that can be applied to both. That the problem appears in both places doesn't surprise me.

This is the progress I have made so far from my understanding and please correct me if I am wrong since I have not much experience in developing in GCC framework.

It sounds like you've got a good understanding of the problem for sure.

In terms of fixing it, it'd be reasonable to reach out to the people who did the GCC and LLVM work for bitmanip and see if they have a solution. It would be good to have something that works for people in the future who find themselves in this situation too.

Certainly the renaming of INSN_CLASS_B_OR_ZBB_OR_ZBP into INSN_CLASS_B_OR_ZBB_OR_ZBP_ZKB example that you give is a reasonable solution, even if it isn't very elegant. Suggesting to the other maintainers that this is what you are planning might make them consider if there is a better way, if you haven't thought of one already yourself.

Cheers,
Ben

@ben-marshall
Copy link
Member

Can this issue be closed @ksyx ?

@ksyx
Copy link
Contributor Author

ksyx commented Jun 11, 2021

If I understood the status correctly, the Zbk* subextensions has already resolved the problem. Thus this issue could be closed. Thanks.

@ksyx ksyx closed this as completed Jun 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Toolchain Something todo with binutils / gcc / LLVM
Projects
None yet
Development

No branches or pull requests

2 participants