#23 structures cannot be passed as function parameters

[Vladimir Támara Patińo]

/* Cannot pass/receive structures as parameters. This
is valid C (not prohibited in C99 standard, accepted by
other compilers).
*/

int g;

struct my {
int x;
int y;
};

int f(struct my var) {
g=var.x+var.y;
}

[Johan Knol]

Logged In: YES
user_id=63512

The documentation clearly states that structures can not be
used in assignments nor can they be passed as function
parameters.
There could be a work around, but for now this is a feature
request, not a bug.

[Vladimir Támara Patińo]

Logged In: YES
user_id=8034

Sorry, this is not a bug but a feature request. It is
documented
in the section 4.6 ANSI-Compliance of the SDCC manual.

[Philipp Klaus Krause]

Increasing priority, since I'm encountering more and more software and libraries that rely on full struct support.

[Philipp Klaus Krause]

Increasing priority, since aggregate and union handling is currently the main issue that prevents us from compiling ANSI C89 / ISO C90 code.

Philipp

[Visenri]

I think this is quite a usefull feature, and i really miss it in this compiler.
I've been investigating the source code to fix this.
After quite a lot of digging, so far I've accomplished the following:

Assignments:
Any size for pure struct-to-struct.
Implemented as memcpy for structs bigger than 8 bytes, else in-line copy (faster and smaller for such low size copies).
Ternary operations.
Only between same types struc-to-struct, functionret-to-functionret.
Function parameters.
Up to 16 bytes (all pushed to stack).
Function return.
Up to 16 bytes.
Return values are allocated the same way as any other data type (STM8):
8-bit: Register A
16-bit: Register X
32-Bit: Registers X and Y
Other sizes: Via hidden pointer argument

16 bytes limitation comes from the size specified in SDCCsymt.h (I've created a definition for that constant, there was no definition, just hard coded magic numbers):

#define SDCC_SYMBOL_REG_INFO_SIZE   16
typedef struct symbol
{
....
  struct reg_info *regs[SDCC_SYMBOL_REG_INFO_SIZE];

Of course, the SDCC_SYMBOL_REG_INFO_SIZE is used where i found it make sense (yes, I had to search for the number 8, previous value, through all source code).

Would this be enough to comply with ANSI C89/ISO C90 code with restrictions?

At least for me, this is a big improvement over current functionality. Almost all practical struct use cases should be covered with this modification.

More details in next post.

[Visenri]

Details about implementation:

The key was to remove all special handling of structs from files like:
SDCCIcode.c
SDCCsymt.c

Replacing IS_AGGREGATE by IS_ARRAY.
Removing some IS_STRUCT conditionals

Also some changes for each port were expected (for STM8 the changes were pretty small):
stm8/gen.c:
Bigreturn only set by size (not by IS_STRUCT).
stm8/ralloc.c:
Replacing IS_AGGREGATE by IS_ARRAY to avoid calling aggrToPtr

I've got it working with STM8 port. I'll have to do more test, and maybe a regression test file, to check if code works ok for every port, testing 8-16-32-64 and 128 bits structs.

Surely I would need some help with the other ports.
Do you see or expect any major issue in my approach or implementing it in all ports?
Any architectural reason why the feature was not implemented until now?

I'll post the patch as soon as I finish my testing with STM8.

[Philipp Klaus Krause]

This seems quite interesting. Some remarks;

1) Currently, assignment is supported by turning it into memcpy(). That is not an effciéncy issue, since backends can choose tohave a built-in memcpy() - some even generate the same code for memcpy() with large third param as for large assignments, and similarly handle memcpy() with small third parameter the same as small assignments. The z80 with its ldir instruction is a good example.
2) Do all regression tests pass for test-stm8?
3) How problematic would it be to lift the 16 byte limit on struct size?
4) Porting this to z80, z180, gbz80, tlcs90, ez80_z80, r2k and r3ka shouldn't be too hard. Their handling of parameters, return values, etc is somewhat similar to stm8.
5) The first backend in SDCC was mcs51. This backend is somewhat different from stm8: Local variables, parameters, etc in many cases are not put onto the stack (since stack access is not that efficient). The pdk14, pdk15 backends behave similar to mcs51 here.
6) It seems no one is working on pic14 and pic16 or knows their architecture well. So making changes there tends to be much harder.
7) How does this change the traditional handling of structs (i.e. struct accesses being done via pointers, local structs never being allocated to registers?
8) Does it work for union, too?
9) We have some regression tests from GCC, currently disabled due to lack of struct parameter support (tests/gcc-torture-execute-20000706-1.c, tests/gcc-torture-execute-20081103-1.c, tests/gcc-torture-execute-920625-1.c, tests/gcc-torture-execute-950607-2.c, tests/gcc-torture-execute-pr44575.c). Do these pass with your changes?

Philipp

[Visenri]

1- No problem, we can choose to limit/decide it at SDCCast level or handle it at port level as you say.
2-Currently I'm trying to fix gcc-torture-execute-20020402-3.c., It fails trying to allocate more than 16 bytes, calling stm8/ralloc.c - regTypeNum
3-The major problem is code using "reg_info" and "nRegs" inside "symbol" structure. Also in STM8 port, code using "bytes" inside "asmop" struct. Some parts assume that nRegs is always withing "reg_info" and "bytes" array. I need someone more familiar with the allocator to touch this code.
4-I also think so.
5 & 6-I will appreciate help in both cases
7-Should not affect normal struct handling, but as I said I'm still trying to fix al regression tests.
8-Not tested, but should work.
9-If I can make all the currently enabled tests work, I'll try those tests.

[Benedikt Freisen]

What is the current status of your efforts?
Are you willing to share a preliminary patch that we could try out?

[Philipp Klaus Krause]

Given how SDCC handles all struct / union via pointers, I guess the easiest way to implement support is via an new PUSH_VALUE_AT_ADDRESS iCode.

[Philipp Klaus Krause]

Experimental support for stm8, z80 and related is implemented in [r13262].

P.S.: In [r13263], this was extended to pdk with --stack-auto.

[Philipp Klaus Krause]

As of [r13347] this should basically work for all ports. Though there are still some problematic corner cases.

All struct parameter tests now pass except for mcs51, hc08, s08, ds390 and mos6502.

[Philipp Klaus Krause]

Support looks good enough to close this ticket now. I've opened tickets for the individual ports that still lack support for stack parameters on the stack: ds390, hc08, s08, mos6502.