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

Add a note for ILP32E #257

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

Conversation

pcwang-thead
Copy link

No description provided.

@@ -260,6 +260,10 @@ boundary. Registers x16-x31 do not participate in the calling convention, so
there are only six argument registers, a0-a5, only two callee-saved registers,
s0-s1, and only three temporaries, t0-t2.

NOTE: As a consequence, stack alignment is 32-bit when passing arguments by stack
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is more than just non-normative text, this is an actual specification change (to match what GCC apparently does) regarding how register pairs are handled

Copy link
Author

@pcwang-thead pcwang-thead Mar 28, 2022

Choose a reason for hiding this comment

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

I agree with you, but I think we should describe it in more details. And since ILP32E(GCC) has been used by some customers, we should be careful of big changes.

There are some discussions three years ago (riscvarchive/riscv-gcc#148), we may:

  • align stack to 8 bytes.
  • or pass arguments with size of 2*XLEN differently and write it in specification document.

I am looking forward to your advice.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would prefer to make a ABI fix on GCC, and add one more rule for require 16 byte stack alignment, but I expect that will happen after psABI 1.0 release.

That would be a ABI incomparable change, but we already put a disclaimer there to notice that's unstable...:P

Copy link
Author

Choose a reason for hiding this comment

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

May I ask why we need 16 bytes stack alignment? would 8 bytes be better if we need to make it an exception?

Copy link
Collaborator

@kito-cheng kito-cheng Mar 28, 2022

Choose a reason for hiding this comment

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

16 byte since the spec allow RV32EQ as valid combination, but I guess I should spend more time to make sure 2*XLEN is enough or we should require 16 byte to make sure RV32EQ is also valid.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks! I will follow up your fantastic works.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That should be 2 * XLEN is enough since more than that will passed in stack.

Copy link
Author

Choose a reason for hiding this comment

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

That makes sense to me.

@kito-cheng
Copy link
Collaborator

I guess we should revise ilp32e part after release to fix those corner issue and F/D support issue on ilp32e.

EABI might going with divination-based, which means EABI will based on the base ABI with some variant instead of a brand new ABI for that.

So I think we should fix ilp32e issue, and that might break the compatibility of old ilp32e stuffs, that means we also need to figure out how to detect and report error for linking incompatible ilp32e version.

@pcwang-thead
Copy link
Author

Based on my experience with LLVM implementation, there is only one issue: the alignment of types (C/C++ types or aggregates ) with size in (XLEN, 2*XLEN]. That results in issues on stack alignment and handling of variadic arguments. For your reference, I think there are two solutions:

  • Force them to align to 4 bytes, which is counterintuitive. This may increase the usage of registers and save some stack spaces. This is what GCC is doing.
  • Force them to align to 8 bytes, which is intuitive but against common stack alignment of 4 bytes. some registers may not be used during passing variadic arguments and stack will be larger. This is LLVM‘s default behavior if we don't change any things.

I don't know if there is any asm directive or attribute that shows the ABI we used, so it may be difficult to detect ilp32e version.

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.

3 participants