Clarification For Empty Struct Handling in C++ about riscv-elf-psabi-doc HOT 7 CLOSED

The main problem with gcc for the testcase is that we assume the in-struct offset is always zero when we have a single float arg. Though now that I look at the issue, I see you already came to the same conclusion and wrote a similar patch.

I think the handling for empty structs should stay the same, as there is no bug there. The text you quoted only applies to an empty struct. It does not apply to an empty struct inside another struct.

If there is an issue, it would be in the text for flattening structures. However, this text already says
Fields containing empty structs or unions are ignored while flattening, even in C++, unless they have nontrivial copy constructors or destructors.
So gcc does seem to be implementing this correctly.

We only flatten an aggregate when it contains at least one float field, and an empty struct plus an integer doesn't qualify, so we don't need any change to the ABI for this case.

Thus all we need to fix is a gcc bug where it is getting the float field offset wrong when we have empty struct plus float. Since this case is already broken, it isn't really an ABI change, it is just a bug fix, and doesn't need any special handling.

from riscv-elf-psabi-doc.

Jim, thanks for the explanation. I admit I hadn't spotted that the flattening only happens when passing floats. I can imagine how that situation has arrived. I'll post a patch for GCC shortly to fix the float offset case.

I wonder if it is worth adding some words to the "Integer Calling Convention" explicitly stating that empty structures are never flattened in the integer only case, and thus when using C++ this will result in empty space being passed around. Though its correct that with no words this should be the assumed default, this makes the integer ABI slightly less efficient than the float ABI, and so isn't (to me anyway) the "obvious" choice.

The Floating Point ABI (see below) describes a strategy for flattening structures in order to more efficiently pass values in floating point registers. Any structure not covered by the Float Point ABI flattening rules is not flattened, and is passed as a standard structure. When considering empty sub-structures in C++, this means that the space associated with the empty structure can be passed in a register.

from riscv-elf-psabi-doc.

I don't see the need to clarify the int docs. The int docs don't do flattening, so we don't need to document that. They also don't use FP regs, and we don't document that either. You usually don't need to document something that doesn't exist.

The int docs say "Aggregates whose total size is no more than XLEN bits are passed in a register, with the fields laid out as though they were passed in memory. " Whether or not one of the fields is an empty structure is irrelevant. It doesn't change how the aggregate is passed.

I don't feel strongly about this though. If other people think this is helpful we can add it into the docs.

from riscv-elf-psabi-doc.

I don't feel particularly strongly about this either. The fact that flattening doesn't happen for the integer ABI seemed clear to me, but once you've spent long enough with the spec it becomes pretty hard to judge what is obvious/non-obvious.

I think something along the lines of the clarification suggested seems ok. I question if it's needed but like Jim don't feel strongly about it.

from riscv-elf-psabi-doc.

I'd suggest erring on the side of parsimony.

from riscv-elf-psabi-doc.

FYI we now coincidentally have a FSF GCC bug report.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89627
Unlike Andrew's testcase, this one looks like real code.

from riscv-elf-psabi-doc.

No one seems in favor of extending the docs here, so I propose closing this.

from riscv-elf-psabi-doc.