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

Using RVE ABI with rv32i. #79

Closed
jim-wilson opened this issue Sep 25, 2018 · 12 comments
Closed

Using RVE ABI with rv32i. #79

jim-wilson opened this issue Sep 25, 2018 · 12 comments

Comments

@jim-wilson
Copy link
Collaborator

The current docs say
The RV32E calling convention may only be used with the RV32E ISA, hence the role of registers x16-x31 and f0-f31 is not defined. A future version of this specification may relax this constraint.

But it is easy to make rve work with rv32i by making the extra registers call clobbered (i.e caller saved). This is how FP regs already work when using for instance the ilp32 ABI with the rv32gc architecture. This is how the proposed EABI works with registers x16-x31. And this is also how the gcc RVE implementation works, though it needed a small bug fix to make a6 and a7 usage depend on the ABI instead of the architecture. So I think this should be allowed.

@aswaterman
Copy link
Contributor

IIRC the only reason we didn't define it was to punt on defining the role of x16-x31, but your proposal is the most natural one. Any comments from @Kevin-Andes @asb @PkmX @kito-cheng ?

@asb
Copy link
Collaborator

asb commented Sep 26, 2018

I think allowing -mabi=ilp32e to be used with -march=rv32i* in this way is the logical approach and can't see a good reason not to allow it.

@kito-cheng
Copy link
Collaborator

LGTM, I heard some people are talking about possibility of extending RV32E into RV32I (e.g. existed RV32E system want to upgrading ISA to RV32I but don't want to rebuild whole SW system.), so I think it make it possible by this spec change.

@PkmX
Copy link
Contributor

PkmX commented Sep 26, 2018

LGTM.

Rather than opening another issue in riscv-c-api-doc, should __riscv_32e be defined with -march=rv32i -mabi=ilp32e? Or should we have another __riscv_32e_abi to differentiate the case?

@aswaterman
Copy link
Contributor

@jim-wilson do you mind contributing a PR that conveys your proposal?

@Kevin-Andes
Copy link

Do we want to plan and think ahead about its relationship with the new coming EABI? Or they will be incompatible anyway so we don’t care?

Thanks.

Regards,
Kevin

@palmer-dabbelt
Copy link
Contributor

@Kevin-Andes IIRC we tried to design something that would be compatible with the fast-interrupts ABI but found that there would be some incompatibilities anyway. Jim would know for sure, though, as he's actually doing the design :)

@PkmX I would be in favor of splitting the flags here. Imagine you're trying to write some assembly function that must adhere to the RV32E ABI, but can be made better somehow by taking advantage of the extra registers in RV32I. This is essentially the same as -march=rv32if -mabi=ilp32 vs -march=rv32if -mabi=ilp32f.

@asb
Copy link
Collaborator

asb commented Sep 27, 2018

@PkmX I would be in favor of splitting the flags here. Imagine you're trying to write some assembly function that must adhere to the RV32E ABI, but can be made better somehow by taking advantage of the extra registers in RV32I. This is essentially the same as -march=rv32if -mabi=ilp32 vs -march=rv32if -mabi=ilp32f.

You can differentiate that case by checking __riscv_flen and __riscv_float_abi_single. It sounds like we want __riscv_abi_e or __riscv_abi_rve or similar. Should we have something like __riscv_embedded as well that is turned on for -march=rv32e?

@jim-wilson
Copy link
Collaborator Author

@palmer-dabbelt , we don't need to split the command line options. We do need to fix the builtin preprocessor macros, since __riscv_32e is defined for the RV32E architecture, but we have nothing defined for the RVE ABI which we need if we allow the RVE ABI to be used with RV32I.

@Kevin-Andes All of the proposals for the embedded ABI are different from the current RVE ABI. The RVE ABI has 6 argument registers. The eabi proposals have 4 argument registers. The eabi proposals change the exact number of saved/temporary registers. Most of the eabi proposals renumber some abi registers to try to make better use of the x8-x15 registers that are required by some compressed instructions.

So we need a new builtin preprocessor macro for the RVE ABI, and one that won't conflict with or be confused with the future embedded ABI. The __riscv_32e_abi mentioned by @PkmX sounds OK to me. Or the __riscv_abi_rve mentioned by @asb. But I don't like __riscv_abi_e or __riscv_embedded because that could be confused with a future embedded eabi. I suppose something like __riscv_abi_ilp32e could work though.

@palmer-dabbelt
Copy link
Contributor

Ya, that was by point: we should have an explicit ABI define for the E ABI, as it's unrelated to the ISA in use. Calling it something like __riscv_abi_e makes sense to me, we could add a __riscv_abi_i as well to be complementary and allow -mabi=ilp32i, but that might be overkill.

We should probably plan out the flags for the embedded/fast-interrupts ABI family as well, so we don't have something too ugly. IIRC I invented a scheme and then Jim convinced me it was horribly broken. I've forgotten the scheme, but remember that I agreed with his opinion on it's brokenness... :)

Maybe we should split this off to sw-dev?

@jim-wilson
Copy link
Collaborator Author

I decided that I like the __riscv_abi_rve suggestion from asb. I have written a gcc patch for that. It is late in the week, so I will wait a few days before I commit in case someone wants to comment on that.

rve-abi-macro.txt

@palmer-dabbelt
Copy link
Contributor

Thanks!

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

No branches or pull requests

7 participants