Re: [PATCH] 2nd try: Add sseregparm function attribute for x86

[Richard Henderson <rth at redhat dot com>]

On Thu, Jun 09, 2005 at 12:03:50AM +0200, Richard Guenther wrote:
> + /* Handle a "sseregparm" attribute;
> +    arguments as in struct attribute_spec.handler.  */
> + static tree
> + ix86_handle_sseregparm_attribute (tree *node, tree name,
> + 				 tree args ATTRIBUTE_UNUSED,
> + 				 int flags ATTRIBUTE_UNUSED,
> + 				 bool *no_add_attrs)
> + {
> +   if (TREE_CODE (*node) != FUNCTION_TYPE
> +       && TREE_CODE (*node) != METHOD_TYPE)
> +     {
> +       warning (OPT_Wattributes, "%qs attribute only applies to functions",
> + 	       IDENTIFIER_POINTER (name));
> +       *no_add_attrs = true;
> +     }
> + 
> +   return NULL_TREE;

This needs to handle all the other calling conventions, and rejecting
combinations that don't make sense.  I guess this could be paired with
regparm or fastcall, since sseregparm doesn't say anything about integer
data, but it certainly can't be paired with cdecl or stdcall.

> +       /* First look if we use SFmode or DFmode at all.  */
> +       if (TYPE_MODE (TREE_TYPE (type)) == SFmode)
> +         use_sfmode = true;
> +       else if (TYPE_MODE (TREE_TYPE (type)) == DFmode)
> +         use_dfmode = true;
> +       for (argt = TYPE_ARG_TYPES (type); argt; argt = TREE_CHAIN (argt))
> +         {
> + 	  if (TYPE_MODE (TREE_VALUE (argt)) == SFmode)
> + 	    use_sfmode = true;
> + 	  else if (TYPE_MODE (TREE_VALUE (argt)) == DFmode)
> + 	    use_dfmode = true;
> + 	}
> + 
> +       if ((use_sfmode && !TARGET_SSE)
> + 	  || (use_dfmode && !TARGET_SSE2))
> +         {
> + 	  if (decl)
> + 	    error ("Calling %qD with attribute sseregparm without "
> + 		   "SSE/SSE2 enabled", decl);
> + 	  else
> + 	    error ("Calling %qT with attribute sseregparm without "
> + 		   "SSE/SSE2 enabled", type);
> + 	  return false;

Your documentation for ssemath says to use x86_64 calling conventions,
and nothing else.  In particular, it does not say "don't pass doubles
in sse registers if sse2 is disabled".  At the *user* level, I *really*
don't like changing the calling convention based on the compiler switches
enabled.  That's a disaster waiting to happen.

We could, if you like, split this attribute into sse1regparm and
sse2regparm, where the former only applies to floats and the later
also applies to doubles.  That would certainly give us the desired
effect from the perspective of the cgraph-promoted functions; I'm
unsure about how often the split would be useful for users.

Previously you mentioned having trouble with SSE1 and passing doubles
in sse registers.  I don't replicate that problem.  With small changes
to your code, I obtain

double __attribute__((sseregparm)) foo(double x) { return x + 1; }
double w;
void bar(void) { w = foo(w); }

foo:
        subl    $12, %esp
        movlps  %xmm0, (%esp)
        fldl    (%esp)
        faddl   .LC1
        fstpl   (%esp)
        movlps  (%esp), %xmm0
        addl    $12, %esp
        ret
bar:
        subl    $4, %esp
        movlps  w, %xmm0
        call    foo
        movlps  %xmm0, w
        addl    $4, %esp
        ret

which is exactly what I expected.

> !   /* Set up the number of SSE registers used for passing SFmode
> !      and DFmode arguments.  Warn for mismatching ABI.  */
> !   if (!cum->maybe_vaarg && !cum->fastcall
> !       && ix86_function_sseregparm (fntype, fndecl))
>       {
> !       cum->sse_nregs = 8;
> !       cum->float_in_sse = true;
>       }

Why the fastcall check?  Unless I've missed something, the conventions
are compatible, except for the special case of float.  And we can easily
declare that sseregparm takes precidence.  It definitely seems weird to 
reject fastcall but not (int)regparm.

Move this up above with the other regparm checks and let the varargs
check suppress the values as with all the rest.



r~