-
Notifications
You must be signed in to change notification settings - Fork 58
Conversation
Calring
commented
Nov 29, 2021
Why are you re-creating PRs every time you make a revision? Reviewers can't easily see whether their comments have been addressed. You may just force-push to your PR branch, no need to delete or do anything else. |
OK.Got it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do not like a paragraph as a line because it is not easy to review. Normally, we break the line after a period, like this:
xxx, xxxxxxx.
xxxxx,xxxxxxx.
xxxxxxx,xxxxxxxx.
xxxxxx,xxxxxxxxxxxxxx.
xxxxxxx,xxxxxxx,xxxxxxxx.
The doc is generally fine. but could it be better with few more polish work?
214860c
to
364a1ba
Compare
Great! I've adjusted them accordingly to your suggestions. If there are still mistakes, please continue to put forward. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left comments about pieces which puzzle me during reading.
docs/LoongArch-ELF-ABI-EN.adoc
Outdated
|
||
... If the argument is integer or pointer type, the argument is passed in GAR. | ||
If no GAR is available, it’s passed on the stack. | ||
When passed in registers or on the stack, integer scalars narrower than GRLEN bits are sign-extended to GRLEN bits. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sign-extension typically refers to signed integers. Are unsigned integers zero extended, or treated sign-extended as if the integer was signed?
For example, is uint8_t
value of 0xFF
passed in as 0x00000000000000FF
or as 0xFFFFFFFFFFFFFFFF
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sign-extension typically refers to signed integers. Are unsigned integers zero extended, or treated sign-extended as if the integer was signed?
For example, is
uint8_t
value of0xFF
passed in as0x00000000000000FF
or as0xFFFFFFFFFFFFFFFF
?
both signed and unsigned are passed as 0xFFFFFFFFFFFFFFFF
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a rationale behind this choice? I think that this design requires extra instructions to properly sign-extend and zero-extend the upper bits of unsigned integers. It just adds an extra overhead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a rationale behind this choice?
This depends on the processor's architecture. Liking the LoongArch64, MIPS64, the most operations of 32-bits's instruction, liking compare, add, multiply.... , have striction on the high-32bits.
I think that this design requires extra instructions to properly sign-extend and zero-extend the upper bits of unsigned integers. It just adds an extra overhead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sign-extension typically refers to signed integers. Are unsigned integers zero extended, or treated sign-extended as if the integer was signed?
For example, isuint8_t
value of0xFF
passed in as0x00000000000000FF
or as0xFFFFFFFFFFFFFFFF
?both signed and unsigned are passed as
0xFFFFFFFFFFFFFFFF
.
The description here is indeed incorrect. Unsigned char is zero extended, that is 0xFF passed in as 0x00000000000000FF. I'll modify this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand that it may be beneficial to widen arguments or return values given the processor architecture. I have seen it done in other ABIs.
However, it is very surprising to me to widen unsigned values via sign-extension as if they were signed. The other ABIs widen the unsigned values via zero-extensions. For example, RISC-V that you took a lot of inspiration from widens unsigned values via zero-extension as far as I can tell.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for jumping in, but I think maybe @shushanhf should confirm with their teammates the actual implemented behavior in the gcc port. If the implementation is primarily derived from the RISC-V port, such behavior is unlikely to be deliberately changed.
|
||
==== Structure | ||
|
||
Empty structures are ignored by C compilers which support them as a non-standard extension(same as union arguments and return values). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this trying to say that empty structures do not occupy any registers or space on stack?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I think so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
. 0 < WOA ≤ GRLEN | ||
|
||
.. The structure has only fixed-point members. | ||
If there is an available GAR, the structure is passed through the GAR by value passing; If no GAR is available, it’s passed on the stack. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means that integers and trivial struct wrappers of integers are not passed the same way. Integers are sign-extented; trivial struct wrappers of integers are not sign-extended (upper bits are undefined).
Is this example correct?
void A(int x); // x is passed in `a0` register, upper 32-bit of `a0` are sign-extended
struct S
{
int f;
};
void B(struct S s); // s is passed in `a0` registers, upper 32-bit of `a0` registers are undefined
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means that integers and trivial struct wrappers of integers are not passed the same way. Integers are sign-extented; trivial struct wrappers of integers are not sign-extended (upper bits are undefined).
Is this example correct?
void A(int x); // x is passed in `a0` register, upper 32-bit of `a0` are sign-extended struct S { int f; }; void B(struct S s); // s is passed in `a0` registers, upper 32-bit of `a0` registers are undefined
There is only one field, the upper 32-bits should be sign extened.
If struct S {char a; cha b;}
, the upper bits:63-16 are also sign extened.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sounds contrary to https://github.com/loongson/LoongArch-Documentation/pull/32/files#diff-711b3e7b6a005b492898ac6d93f2d8d37c00e0831e210993a6f9dbb26c043717R699 : Bits unused due to padding, and bits past the end of a structure whose size in bits is not divisible by GRLEN, are undefined.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is only one field, the upper 32-bits should be sign extened.
This's right.
If struct S {char a; char b;} , the upper bits:63-16 are also sign extened.
The upper bits are undefiened
docs/LoongArch-ELF-ABI-EN.adoc
Outdated
|
||
.. Only float-point members. | ||
|
||
... i.e. The structure has one long double member or one double member and two adjacent float members or 3-4 float members. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this also apply to a structure that has two double
members? Is structure with two double
members passed in GARs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this also apply to a structure that has two
double
members? Is structure with twodouble
members passed in GARs?
Yes, the struct is passed by two integer register(GARs).
By the way, I had asked the author that ... i.e. The structure has one long double member or one double member and two adjacent float members or 3-4 float members.
He wanted to express that:
struct {long double a;} or struct {double a; double b;} or struct {double a; float b; float c;} or struct {float a; float b; float c; float d;}.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the only case of GRLEN < WOA ≤ 2 × GRLEN structures that is candidate for FAR is one double member and one fixed-point member. All of other cases are passed in GAR. I think that the description can be simplified and just mention this one case as an exception.
Is there a rationale for why the one double member and one fixed-point member structures are special cased like this? It sounds like a lot of complexity for very little gain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the only case of GRLEN < WOA ≤ 2 × GRLEN structures that is candidate for FAR is one double member and one fixed-point member. All of other cases are passed in GAR. I think that the description can be simplified and just mention this one case as an exception.
Yes, there is room for improment.
Is there a rationale for why the one double member and one fixed-point member structures are special cased like this? It sounds like a lot of complexity for very little gain.
Now it's too late for amendment of the ABI. (By the way, the RISC-V has the same situation)
This ABI for gcc/llvm might be unconspicuous, but for JIT is a lot fo comlexity and maybe worse for gain.
For dotnet-runtime, I have to implement the ABI following this spec only.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the only case of GRLEN < WOA ≤ 2 × GRLEN structures that is candidate for FAR is one double member and one fixed-point member. All of other cases are passed in GAR. I think that the description can be simplified and just mention this one case as an exception.
Is there a rationale for why the one double member and one fixed-point member structures are special cased like this? It sounds like a lot of complexity for very little gain.
In order to be consistent with the classification above, and it looks elegant. It does look a little more complicated, but I think it's easy to understand after read it.
We will consider your suggestion seriously. Thank you very much.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this also apply to a structure that has two
double
members? Is structure with twodouble
members passed in GARs?
Actually, a structure with two double
members is passed in pair of available FARS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this also apply to a structure that has two
double
members? Is structure with twodouble
members passed in GARs?
Sorry for missing this. I've added it in new submit.
de5b026
to
2ef0e1e
Compare
docs/LoongArch-ELF-ABI-EN.adoc
docs/LoongArch-ELF-ABI-EN.adoc
8ad984e
to
3f976fb
Compare
docs/LoongArch-ELF-ABI-EN.adoc
docs/LoongArch-ELF-ABI-EN.adoc
docs/LoongArch-ELF-ABI-EN.adoc
1. The GRLEN < WOA <= 2GRLEN part of structure. 2. The GRLEN < WOA <= 2GRLEN part of variadic arguments. 3. Other modifications. docs/LoongArch-ELF-ABI-EN.adoc