Ping^5: Re: Updated^2: RFA: Fix middle-end/46500 (void * encapsulated)

Bernd Schmidt bernds@codesourcery.com
Tue Jun 14 13:43:00 GMT 2011


On 06/14/2011 02:53 PM, Joern Rennecke wrote:
> Quoting Bernd Schmidt <bernds@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



More information about the Gcc-patches mailing list