Re: [patch i386]: Enable ms_abi/sysv_abi and add testcases

[Kai Tietz <Kai dot Tietz at onevision dot com>]

> 2008/6/17 Joseph S. Myers <joseph@codesourcery.com>:
> > On Tue, 17 Jun 2008, Kai Tietz wrote:
> >
> >> May the name could be better, but it makes absolutely sense! The 
va_list
> >> type is call abi dependent. It makes absolutely sense to do this as 
macro,
> >> so targets, which have just one va_list abi have no disadvantage (and 
need
> >> no change). Secondly, the type of va_list is always specified proper 
via
> >> the global cfun.
> >
> > Then you need to document that it's for the current function, but I 
think
> > it would be better to pass in the function explicitly.  Furthermore, 
the
> > ABI of the current function is relevant for a va_start call - but it's 
not
> > relevant for a va_arg call; there you want to know the ABI of the 
va_list
> > passed to va_arg, which might have been initialized by a va_start call 
in
> > a different function with a different ABI.  And it's only relevant for 
a
> > va_start call to check that the va_list passed is one for that 
function's
> > ABI rather than another ABI - you could choose to allow va_start with 
a
> > va_list for the other ABI, to create such a va_list to pass to a 
function
> > expecting other-ABI va_list (if this is possible with the ABIs in
> > question).
> 
> Well, the major problem here is the difference between ARRAY typed
> va_list and stack reference version. If we would change
> va_start/va_end to react on ARRAY/stack ref. type, a lot of sanity
> checks have to be removed. The other problem I see is, that who to
> toggle in builtin between the two (or more) different va_list types?
> 
> > I think there are at least three things involved:
> >
> > (a) The ABI of a particular variadic function, for which va_start is 
being
> > called.
> >
> > (b) The ABI of a particular va_list object, for which any va_* macro 
is
> > being called
> >
> > (c) The ABI of a built-in function implementing a va_* macro, if you
> > require an ABI-specific function to be used rather than making them
> > ABI-generic.
> >
> > The consistency needed is that (c) needs to agree with (b) in each 
call,
> > and (b) needs to agree with (a) if the ABIs make it impossible to 
create a
> > foreign va_list for the arguments to a variadic function call (but not 
if
> > the ABIs are similar enough to allow such a va_list to be created). If
> > the built-in functions are made ABI-generic, then (c) can be deduced 
from
> > (b) and all that's needed is that the object has one of the valid 
va_list
> > types.
> 
> By the current implementation it save, that you can't use
> va_start/va_end/va_copy to an none call-abi va_list type. This one is
> save, because the internal checks are using the current abi specific
> builtin type to verify. So a use of a *foreign* va_list type leads to
> an error.
> 
> 
> >> May you can point me to an variant of how to implement this - without 
the
> >> need to touch all targets and compile time punishing all targets with 
just
> >> one va_list type.
> >
> > The transition to target hooks is concerned with cleanliness of the 
code
> > rather than micro-optimization (we sort-of hope that one day LTO will
> > allow target hooks to be inlined).  I'd be surprised if va_list checks 
are
> > done enough for the overhead of calling a function that does a 
comparison
> > instead of doing the comparison directly to be measurable.
> 
> Ok, I'll modify my patch to use a target hook instead.

I changed the patch to use a new target hook. I named it 
TARGET_CFUN_ABI_VA_LIST. This is a bit better then the last name I used.

ChangeLog

2008-06-18  Kai Tietz  <kai.tietz@onevision.com>

        * builtins.c (std_cfun_abi_va_list): New.
        (stabilize_va_list): Replace va_list_type_node use by
        mtarget.cfun_abi_va_list.
        (gimplify_va_arg_expr): Likewise.
        (expand_builtin_va_copy): Likewise.
        * tree-sra.c (decl_can_be_decomposed_p): Likewise.
        * tree-ssa-ccp.c (optimize_stdarg_builtin): Likewise.
        * tree-stdarg.c (execute_optimize_stdarg): Likewise.
        * c-common.c (c_common_nodes_and_builtins): Use 
TARGET_ENUM_VA_LIST.
        * config/i386/i386-protos.h (ix86_get_valist_type): New.
        (ix86_enum_va_list): New.
        * config/i386/i386.c (sysv_va_list_type_node): New.
        (ms_va_list_type_node): New.
        (ix86_function_type_abi): Remove sorry.
        (ix86_build_builtin_va_list_abi): New.
        (ix86_build_builtin_va_list): Call ix86_build_builtin_va_list_abi
        for 64-bit targets.
        (ix86_va_start): Replace va_list_type_node by 
sysv_va_list_type_node.
        (build_function_type_list_ellipsis): New helper.
        (ix86_init_builtins_va_builtins_abi): New.
        (ix86_init_builtins): Use ix86_init_builtins_va_builtins_abi
        for 64-bit targets.
        (ix86_handle_abi_attribute): New.
        (attribute_spec): Add sysv_abi and ms_abi.
        (ix86_cfun_abi_va_list): New.
        (ix86_enum_va_list): New.
        (TARGET_CFUN_ABI_VA_LIST): New.
        * config/i386/i386.h (TARGET_ENUM_VA_LIST): New.
        * doc/tm.texi (TARGET_CFUN_ABI_VA_LIST): New.
        (TARGET_ENUM_VA_LIST): New.
        * expr.h (std_cfun_abi_va_list): New.
        * target-def.h (TARGET_CFUN_ABI_VA_LIST): New.
        (TARGET_INITIALIZER): Add TARGET_CFUN_ABI_VA_LIST.
        * target.h (struct gcc_target): Add cfun_abi_va_list hook.
 

2008-06-18  Kai Tietz  <kai.tietz@onevision.com>

        * gcc.dg/callabi/callabi.h: New.
        * gcc.dg/callabi/vaarg-1.c: New.
        * gcc.dg/callabi/func-1.c: New.

I fixed some typos in testcase callabi.h. Another interesting issue I 
found is that func-1.c fails if fast-math isn't enabled. So I added this 
switch to this testcase.

Is this patch ok for apply to trunk?

--
Kai Tietz


|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

Attachment: callabi_tstcase.txt
Description: Text document