Re: [PATCH] MALLOC_ABI_ALIGNMENT macro + malloc-alignment param

["Richard Guenther" <richard dot guenther at gmail dot com>]

On Fri, Jul 18, 2008 at 5:05 PM, Olivier Hainque <hainque@adacore.com> wrote:
> Richard Guenther wrote:
>> ... as we detect malloc by means of the malloc attribute and existing
>> usage includes things such as small-object allocators (which do _not_
>> need to follow the above, but may return 2-byte or 1-byte aligned
>> storage), we have to invent a new mechanism to specify alignment
>> of an allocator function (maybe your patch did that already, I don't
>> remember).
>
>  No, the patch didn't do that. It is a very simple helper to our
>  current scheme:
>
>  For a type T without a dedicated Ada storage pool, 'new T' eventually
>  maps to a call to "__gnat_malloc", via gigi's build_allocator.
>
>  The latter crafts a special "aligning" type to allocate when the user
>  type alignment is beyond what __gnat_malloc offers, which we currently
>  assume honors at least as much as the system's malloc.
>
>  We have local static definitions of these values for some targets,
>  a potentially incorrect default to BIGGEST_ALIGNMENT, and the initial
>  patch was meant to start a discussion on if/how to adjust and integrate
>  these. This is indeed a typical target ABI thing, and GCC's infrastructure
>  for target parameters is a pretty natural place to locate this.
>
>  I agree that the static values need to be refined, and the param idea
>  looked like a very useful complement to deal with cases of a user
>  redefined "default allocator", aka __gnat_malloc in the Ada case (users
>  are allowed to do that by providing an alternate System.Memory package
>  body).
>
>> I would suggest a new malloc_align attribute which we can set to the
>> standard C allocators appropriately with values derived from the
>> target ABI.
>
>  This sounds like a useful device in general. I'm however afraid that
>  it wouldn't help our immediate purposes without potentially significant
>  extra work, because __gnat_malloc's decl isn't parsed with the
>  processed units but handcrafted by gigi.
>
>> >  As your malloc(8) example illustrates, we have to cut at some lower
>> >  bound in any case.
>> >
>> >  How would you compute that ?
>>
>> The target ABI specifies that.
>
>  OK, we're on the same page here :)
>
>> >  while safe, 1 would be overly pessimistic.
>>
>> Well, you might call it overly pessimistic - I call it correct.
>
>  I agree it's correct :) Just saying that what "the target ABI
>  specifies" would almost certainly be at least slightly larger, be
>  correct as well, and that if we don't account for it, performance
>  will drop.
>
>  MIN(STACK_BOUNDARY,2*WORD) in the previous suggestion was simply
>  meant as a default expression for "what the target ABI specifies",
>  to be overriden on a case by case basis on targets where it's
>  incorrect.
>
>  We could make it 1, or maybe INTEGER_SIZE, to be correct by default
>  and override the other way around.

Ok.  I remember your patch now.  I suggest to add a target macro that specifies
the minimal alignment a C conformant malloc implementation has to provide.
This would for example be 4 on i?86-linux (even though the glibc implementation
provides 8 I think) and 8 on x86_64-linux.  The default value for the
target macro
should be 1 for safety, though eventually 4 for ilp32 and 8 for lp64
targets would
do as well.

The malloc_align function attribute is a separate thing and you don't need to
bother about it.  I don't think we should have a --param to override the values.

Richard.