On 06/14/2011 02:53 PM, Joern Rennecke wrote: > Quoting Bernd Schmidt <ber...@codesourcery.com>: > >> I'm not getting the point of the use of attribute((transparent_union)). > > Without that attribute, lots of ABIs add a lot of overhead for function > argument and return value passing.
* These functions are not hotspots. * Most sane ABIs pass single-word structs in registers * For the most part, gcc runs on i686 and there it doesn't make a difference. If ARM takes over the world, it still does not make a difference. This is an unnecessary and premature microoptimization. Please remove it. >> and to eliminate a potential source of bugs when >> passing cumulative_args_t arguments. > > Is that about not trusting the bootstrap gcc to implement that attribute > correctly? > Or do you want the integrity check from the ENABLE_CHECKING case to be > always present? According to the transparent union documentation, the compiler will accept void * rather than cumulative_args_t if the latter is declared as a transparent union. If the point of your ENABLE_CHECKING machinery (which I also don't really understand) is to avoid exactly that kind of bug, then the ENABLE_CHECKING code should go away along with the use of transparent_union. > [fr30.c:fr30_setup_incoming_varargs] >> - if (targetm.calls.strict_argument_naming (arg_regs_used_so_far)) >> + /* ??? the code inside is a pointer increment. */ >> + if (targetm.calls.strict_argument_naming (arg_regs_used_so_far_v)) >> >> What does this comment mean? > > It means that the code inside the if clause. i.e.: > > arg_regs_used_so_far += fr30_num_arg_regs (mode, type); > > is nonsense. arg_regs_used_so_far is a pointer to int. The pointed-to int > is supposed to record the number of words used for argument passing. > The statement increments the pointer. Ok, so move the comment before that statement then and adjust it to say this. Bernd
