[PATCH] ARM half-precision floating point, 3/8 (target hooks)

[PATCH] ARM half-precision floating point, 3/8 (target hooks)

[Sandra Loosemore]

[-- Attachment #1: Type: text/plain, Size: 1476 bytes --]

This patch adds the target hooks that are required to get the C and
C++ language-level support for HFmode, specifically including the
implicit promotion and restrictions on function argument/return types.
The included documentation patch should be sufficient to explain the
details.

-Sandra


2009-04-15  Sandra Loosemore  <sandra@codesourcery.com>

	gcc/
	* doc/tm.texi (Misc): Document TARGET_INVALID_PARAMETER_TYPE,
	TARGET_INVALID_RETURN_TYPE, TARGET_PROMOTED_TYPE, and
	TARGET_CONVERT_TO_TYPE.
	* hooks.c (hook_tree_const_tree_null): Define.
	* hooks.h (hook_tree_const_tree_null): Declare.
	* target.h (struct gcc_target):  Add invalid_parameter_type,
	invalid_return_type, promoted_type, and convert_to_type fields.
	* target-def.h: (TARGET_INVALID_PARAMETER_TYPE): Define.
	(TARGET_INVALID_RETURN_TYPE): Define.
	(TARGET_PROMOTED_TYPE): Define.
	(TARGET_CONVERT_TO_TYPE): Define.
	(TARGET_INITIALIZER): Update for new fields.
	* c-decl.c (grokdeclarator): Check targetm.invalid_return_type.
	(grokparms): Check targetm.invalid_parameter_type.
	* c-typeck.c (default_conversion): Check targetm.promoted_type.
	* c-convert.c (convert): Check targetm.convert_to_type.
	* cp/typeck.c (default_conversion): Check targetm.promoted_type.
	* cp/decl.c (grokdeclarator): Check targetm.invalid_return_type.
	(grokparms): Check targetm.invalid_parameter_type.
	* cp/cvt.c (ocp_convert): Check targetm.convert_to_type.
	(build_expr_type_conversion): Check targetm.promoted_type.

[-- Attachment #2: fp16-3.patch --]
[-- Type: text/x-patch, Size: 11291 bytes --]

Index: gcc/doc/tm.texi
===================================================================
--- gcc/doc/tm.texi	(revision 146010)
+++ gcc/doc/tm.texi	(working copy)
@@ -10684,6 +10684,36 @@ and @var{type2}, or @code{NULL} if valid
 the front end.
 @end deftypefn
 
+@deftypefn {Target Hook} {const char *} TARGET_INVALID_PARAMETER_TYPE (tree @var{type})
+If defined, this macro returns the diagnostic message when it is
+invalid for functions to include parameters of type @var{type}, 
+or @code{NULL} if validity should be determined by
+the front end.
+@end deftypefn
+
+@deftypefn {Target Hook} {const char *} TARGET_INVALID_RETURN_TYPE (tree @var{type})
+If defined, this macro returns the diagnostic message when it is
+invalid for functions to have return type @var{type}, 
+or @code{NULL} if validity should be determined by
+the front end.
+@end deftypefn
+
+@deftypefn {Target Hook} {tree} TARGET_PROMOTED_TYPE (tree @var{type})
+If defined, this target hook returns the type to which values of 
+@var{type} should be promoted when they appear in expressions, 
+analogous to the integer promotions, or @code{NULL_TREE} to use the
+front end's normal promotion rules.  This hook is useful when there are
+target-specific types with special promotion rules.
+@end deftypefn
+
+@deftypefn {Target Hook} {tree} TARGET_CONVERT_TO_TYPE (tree @var{type}, tree @var{expr})
+If defined, this hook returns the result of converting @var{expr} to 
+@var{type}.  It should return the converted expression, 
+or @code{NULL_TREE} to apply the front end's normal conversion rules.
+This hook is useful when there are target-specific types with special 
+conversion rules.
+@end deftypefn
+
 @defmac TARGET_USE_JCR_SECTION
 This macro determines whether to use the JCR section to register Java
 classes. By default, TARGET_USE_JCR_SECTION is defined to 1 if both
Index: gcc/hooks.c
===================================================================
--- gcc/hooks.c	(revision 146010)
+++ gcc/hooks.c	(working copy)
@@ -335,3 +335,10 @@ hook_constcharptr_int_const_tree_const_t
 {
   return NULL;
 }
+
+/* Generic hook that takes a const_tree and returns NULL_TREE.  */
+tree
+hook_tree_const_tree_null (const_tree t ATTRIBUTE_UNUSED)
+{
+  return NULL;
+}
Index: gcc/hooks.h
===================================================================
--- gcc/hooks.h	(revision 146010)
+++ gcc/hooks.h	(working copy)
@@ -64,6 +64,8 @@ extern int hook_int_rtx_bool_0 (rtx, boo
 extern int hook_int_size_t_constcharptr_int_0 (size_t, const char *, int);
 extern int hook_int_void_no_regs (void);
 
+extern tree hook_tree_const_tree_null (const_tree);
+
 extern tree hook_tree_tree_tree_null (tree, tree);
 extern tree hook_tree_tree_tree_tree_null (tree, tree, tree);
 extern tree hook_tree_tree_tree_tree_3rd_identity (tree, tree, tree);
Index: gcc/target.h
===================================================================
--- gcc/target.h	(revision 146010)
+++ gcc/target.h	(working copy)
@@ -901,6 +901,24 @@ struct gcc_target
      is not permitted on TYPE1 and TYPE2, NULL otherwise.  */
   const char *(*invalid_binary_op) (int op, const_tree type1, const_tree type2);
 
+  /* Return the diagnostic message string if TYPE is not valid as a
+     function parameter type, NULL otherwise.  */
+  const char *(*invalid_parameter_type) (const_tree type);
+
+  /* Return the diagnostic message string if TYPE is not valid as a
+     function return type, NULL otherwise.  */
+  const char *(*invalid_return_type) (const_tree type);
+
+  /* If values of TYPE are promoted to some other type when used in
+     expressions (analogous to the integer promotions), return that type,
+     or NULL_TREE otherwise.  */
+  tree (*promoted_type) (const_tree type);
+
+  /* Convert EXPR to TYPE, if target-specific types with special conversion
+     rules are involved.  Return the converted expression, or NULL to apply
+     the standard conversion rules.  */
+  tree (*convert_to_type) (tree type, tree expr);
+
   /* Return the array of IRA cover classes for the current target.  */
   const enum reg_class *(*ira_cover_classes) (void);
 
Index: gcc/cp/typeck.c
===================================================================
--- gcc/cp/typeck.c	(revision 146010)
+++ gcc/cp/typeck.c	(working copy)
@@ -1699,10 +1699,14 @@ decay_conversion (tree exp)
 tree
 default_conversion (tree exp)
 {
+  /* Check for target-specific promotions.  */
+  tree promoted_type = targetm.promoted_type (TREE_TYPE (exp));
+  if (promoted_type)
+    exp = cp_convert (promoted_type, exp);
   /* Perform the integral promotions first so that bitfield
      expressions (which may promote to "int", even if the bitfield is
      declared "unsigned") are promoted correctly.  */
-  if (INTEGRAL_OR_UNSCOPED_ENUMERATION_TYPE_P (TREE_TYPE (exp)))
+  else if (INTEGRAL_OR_UNSCOPED_ENUMERATION_TYPE_P (TREE_TYPE (exp)))
     exp = perform_integral_promotions (exp);
   /* Perform the other conversions.  */
   exp = decay_conversion (exp);
Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c	(revision 146010)
+++ gcc/cp/decl.c	(working copy)
@@ -7607,6 +7607,7 @@ grokdeclarator (const cp_declarator *dec
   bool parameter_pack_p = declarator? declarator->parameter_pack_p : false;
   bool set_no_warning = false;
   bool template_type_arg = false;
+  const char *errmsg;
 
   signed_p = declspecs->specs[(int)ds_signed];
   unsigned_p = declspecs->specs[(int)ds_unsigned];
@@ -8286,6 +8287,12 @@ grokdeclarator (const cp_declarator *dec
 		type_quals = TYPE_UNQUALIFIED;
 		set_no_warning = true;
 	      }
+	    errmsg = targetm.invalid_return_type (type);
+	    if (errmsg)
+	      {
+		error (errmsg);
+		type = integer_type_node;
+	      }
 
 	    /* Error about some types functions can't return.  */
 
@@ -9665,6 +9672,7 @@ grokparms (tree parmlist, tree *parms)
       tree type = NULL_TREE;
       tree init = TREE_PURPOSE (parm);
       tree decl = TREE_VALUE (parm);
+      const char *errmsg;
 
       if (parm == void_list_node)
 	break;
@@ -9698,6 +9706,14 @@ grokparms (tree parmlist, tree *parms)
 	  init = NULL_TREE;
 	}
 
+      if (type != error_mark_node
+	  && (errmsg = targetm.invalid_parameter_type (type)))
+	{
+	  error (errmsg);
+	  type = error_mark_node;
+	  TREE_TYPE (decl) = error_mark_node;
+	}
+
       if (type != error_mark_node)
 	{
 	  if (deprecated_state != DEPRECATED_SUPPRESS)
Index: gcc/cp/cvt.c
===================================================================
--- gcc/cp/cvt.c	(revision 146010)
+++ gcc/cp/cvt.c	(working copy)
@@ -581,6 +581,7 @@ ocp_convert (tree type, tree expr, int c
   tree e = expr;
   enum tree_code code = TREE_CODE (type);
   const char *invalid_conv_diag;
+  tree e1;
 
   if (error_operand_p (e) || type == error_mark_node)
     return error_mark_node;
@@ -629,6 +630,9 @@ ocp_convert (tree type, tree expr, int c
 	}
     }
 
+  if ((e1 = targetm.convert_to_type (type, e)))
+    return e1;
+
   if (code == VOID_TYPE && (convtype & CONV_STATIC))
     {
       e = convert_to_void (e, /*implicit=*/NULL, tf_warning_or_error);
@@ -1224,11 +1228,18 @@ build_expr_type_conversion (int desires,
 tree
 type_promotes_to (tree type)
 {
+  tree promoted_type;
+
   if (type == error_mark_node)
     return error_mark_node;
 
   type = TYPE_MAIN_VARIANT (type);
 
+  /* Check for promotions of target-defined types first.  */
+  promoted_type = targetm.promoted_type (type);
+  if (promoted_type)
+    return promoted_type;
+
   /* bool always promotes to int (not unsigned), even if it's the same
      size.  */
   if (type == boolean_type_node)
Index: gcc/c-decl.c
===================================================================
--- gcc/c-decl.c	(revision 146010)
+++ gcc/c-decl.c	(working copy)
@@ -4000,6 +4000,7 @@ grokdeclarator (const struct c_declarato
   bool bitfield = width != NULL;
   tree element_type;
   struct c_arg_info *arg_info = 0;
+  const char *errmsg;
   tree expr_dummy;
   bool expr_const_operands_dummy;
 
@@ -4583,6 +4584,12 @@ grokdeclarator (const struct c_declarato
 		error ("%qs declared as function returning an array", name);
 		type = integer_type_node;
 	      }
+	    errmsg = targetm.invalid_return_type (type);
+	    if (errmsg)
+	      {
+		error (errmsg);
+		type = integer_type_node;
+	      }
 
 	    /* Construct the function type and go to the next
 	       inner layer of declarator.  */
@@ -5096,6 +5103,7 @@ grokparms (struct c_arg_info *arg_info, 
     {
       tree parm, type, typelt;
       unsigned int parmno;
+      const char *errmsg;
 
       /* If there is a parameter of incomplete type in a definition,
 	 this is an error.  In a declaration this is valid, and a
@@ -5139,6 +5147,14 @@ grokparms (struct c_arg_info *arg_info, 
 		}
 	    }
 
+	  errmsg = targetm.invalid_parameter_type (type);
+	  if (errmsg)
+	    {
+	      error (errmsg);
+	      TREE_VALUE (typelt) = error_mark_node;
+	      TREE_TYPE (parm) = error_mark_node;
+	    }
+
 	  if (DECL_NAME (parm) && TREE_USED (parm))
 	    warn_if_shadowing (parm);
 	}
Index: gcc/c-typeck.c
===================================================================
--- gcc/c-typeck.c	(revision 146010)
+++ gcc/c-typeck.c	(working copy)
@@ -1780,6 +1780,7 @@ default_conversion (tree exp)
   tree orig_exp;
   tree type = TREE_TYPE (exp);
   enum tree_code code = TREE_CODE (type);
+  tree promoted_type;
 
   /* Functions and arrays have been converted during parsing.  */
   gcc_assert (code != FUNCTION_TYPE);
@@ -1807,6 +1808,10 @@ default_conversion (tree exp)
   if (exp == error_mark_node)
     return error_mark_node;
 
+  promoted_type = targetm.promoted_type (type);
+  if (promoted_type)
+    return convert (promoted_type, exp);
+
   if (INTEGRAL_TYPE_P (type))
     return perform_integral_promotions (exp);
 
Index: gcc/c-convert.c
===================================================================
--- gcc/c-convert.c	(revision 146010)
+++ gcc/c-convert.c	(working copy)
@@ -86,6 +86,8 @@ convert (tree type, tree expr)
 
   if (type == TREE_TYPE (expr))
     return expr;
+  if ((ret =  targetm.convert_to_type (type, expr)))
+      return ret;
 
   STRIP_TYPE_NOPS (e);
 
Index: gcc/target-def.h
===================================================================
--- gcc/target-def.h	(revision 146010)
+++ gcc/target-def.h	(working copy)
@@ -537,6 +537,10 @@
 #define TARGET_INVALID_CONVERSION hook_constcharptr_const_tree_const_tree_null
 #define TARGET_INVALID_UNARY_OP hook_constcharptr_int_const_tree_null
 #define TARGET_INVALID_BINARY_OP hook_constcharptr_int_const_tree_const_tree_null
+#define TARGET_INVALID_PARAMETER_TYPE hook_constcharptr_const_tree_null
+#define TARGET_INVALID_RETURN_TYPE hook_constcharptr_const_tree_null
+#define TARGET_PROMOTED_TYPE hook_tree_const_tree_null
+#define TARGET_CONVERT_TO_TYPE hook_tree_tree_tree_null
 
 #define TARGET_FIXED_CONDITION_CODE_REGS hook_bool_uintp_uintp_false
 
@@ -918,6 +922,10 @@
   TARGET_INVALID_CONVERSION,			\
   TARGET_INVALID_UNARY_OP,			\
   TARGET_INVALID_BINARY_OP,			\
+  TARGET_INVALID_PARAMETER_TYPE,		\
+  TARGET_INVALID_RETURN_TYPE,			\
+  TARGET_PROMOTED_TYPE,				\
+  TARGET_CONVERT_TO_TYPE,			\
   TARGET_IRA_COVER_CLASSES,			\
   TARGET_SECONDARY_RELOAD,			\
   TARGET_EXPAND_TO_RTL_HOOK,			\

Re: [PATCH] ARM half-precision floating point, 3/8 (target hooks)

[Paolo Bonzini]

Sandra Loosemore wrote:
> This patch adds the target hooks that are required to get the C and
> C++ language-level support for HFmode, specifically including the
> implicit promotion and restrictions on function argument/return types.
> The included documentation patch should be sufficient to explain the
> details.

Is this necessary at all?  Can we define HFmode to be a storage-only
type at the language level?

(Just wondering.  The series looks great).

Paolo

Re: [PATCH] ARM half-precision floating point, 3/8 (target hooks)

[Sandra Loosemore]

Paolo Bonzini wrote:
> Sandra Loosemore wrote:
>> This patch adds the target hooks that are required to get the C and
>> C++ language-level support for HFmode, specifically including the
>> implicit promotion and restrictions on function argument/return types.
>> The included documentation patch should be sufficient to explain the
>> details.
> 
> Is this necessary at all?  Can we define HFmode to be a storage-only
> type at the language level?

It would be nice if we could do that.  I imagine that now that half-precision is 
in the IEEE standard, it's going to be picked up by other targets and may make 
it into future versions of the C/C++ language standards as well (although maybe 
not with the exact semantics that I've implemented here).  I thought that the 
language-level issues were quite straightforward, both conceptually and 
implementation-wise.

OTOH, it was a lot of work to retro-fit the ARM back end to know about HFmode, 
and most of it was not even related to the 2 new instructions.  (The constant 
pool handling gave us fits, for instance.)  I clearly do not have either the 
time or target-specific knowledge to retrofit all the other back ends, too, and 
it's not realistic to think somebody else is going to do all that work.  ;-)  So 
if we did add HFmode knowledge to the compiler front end, we'd still need some 
sort of target hook to control whether it's supported at all by the back end.

Anyway, I could rework this part of the implementation if there's consensus that 
it's the right thing to do, that the semantics as currently implemented for ARM 
are useful in general, etc.  I'd have to give it some thought to come up with a 
new design for the front end/back end interface, though -- so I'd prefer to get 
some guidance that it's really the right direction before diving into it.

-Sandra

Re: [PATCH] ARM half-precision floating point, 3/8 (target hooks)

[Paolo Bonzini]

> It would be nice if we could do that.  I imagine that now that
> half-precision is in the IEEE standard, it's going to be picked up by
> other targets and may make it into future versions of the C/C++ language
> standards as well (although maybe not with the exact semantics that I've
> implemented here).

Yes -- and if the semantics change in the standards, ARM is probably
going to adjust.  Having at least one compiler "attempt" to provide a
target-independent extension is also good from a standards body point of
view.

> I thought that the language-level issues were quite
> straightforward, both conceptually and implementation-wise.
> 
> So if we did add HFmode knowledge to
> the compiler front end, we'd still need some sort of target hook to
> control whether it's supported at all by the back end.

There are two such knobs already in place.  One is simply
register_builtin_type, as you use it in arm_init_fp16_builtins; this
makes it harder to "casually" use HFmode if it is not supported.

The other is the targetm.scalar_mode_supported_p hook, which is used to
handle something like

  typedef float fp16 __attribute__((mode (HF)));

Right now, this would fail even for ARM, but this is not something you
need to fix now.

> Anyway, I could rework this part of the implementation if there's
> consensus that it's the right thing to do, that the semantics as
> currently implemented for ARM are useful in general, etc.

Yes, that's right, but I think that the semantics are quite nice and
unambiguous from what I saw in the code.  It is feasible to foresee the
lifting of the restrictions on parameter passing and returning (after
all you can pass and return char, and all that does is inserting
coercions on the callee resp. caller side), but that's easy to do in the
future and does not break backwards compatibility.

There is one point that I did not really understand, which is the
convert_to_type hook, since you are then implementing the same semantics
again in the RTL patterns for fix_trunchfsi and floathfsi2.  Everything
else could simply be inlined at the point where you call the target hook.

> I'd have to
> give it some thought to come up with a new design for the front end/back
> end interface, though -- so I'd prefer to get some guidance that it's
> really the right direction before diving into it.

I don't think that there is any interface needed more than what is
already in the compiler.

Paolo

Re: [PATCH] ARM half-precision floating point, 3/8 (target hooks)

[Joseph S. Myers]

On Thu, 16 Apr 2009, Paolo Bonzini wrote:

> Sandra Loosemore wrote:
> > This patch adds the target hooks that are required to get the C and
> > C++ language-level support for HFmode, specifically including the
> > implicit promotion and restrictions on function argument/return types.
> > The included documentation patch should be sufficient to explain the
> > details.
> 
> Is this necessary at all?  Can we define HFmode to be a storage-only
> type at the language level?

It's not clear to me that we have enough cases of such types to be able to 
tell what should or should not be common between the implementations for 
different architectures.  Hence the target hooks, with the possibility of 
moving more directly to the front ends in future if several targets want 
the same thing.

The function argument and return type restrictions, and conversions from 
double to HFmode going via float (with double rounding), are not things 
that it's clear other targets will want; if some C and C++ interfaces to 
half precision are standardised in future it seems likely they'd avoid 
those restrictions; likewise the lack of _Complex __fp16.  (Indeed the ARM 
EABI defines how half-precision arguments and return types should work at 
the language-independent ABI level; it's just the C and C++ bindings that 
don't allow them.)  If there is future standardisation it would seem 
reasonable for the ARM implementation to align with it, possibly removing 
some hooks.

IA64 __fpreg could equally be considered a storage-only type - it meets 
that description, but has rather different restrictions and hence uses a 
different set of hooks.  I think that illustrates the difficulty of 
defining a common "storage-only" notion at this point.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] ARM half-precision floating point, 3/8 (target hooks)

[Paolo Bonzini]

> The function argument and return type restrictions, and conversions from 
> double to HFmode going via float (with double rounding), are not things 
> that it's clear other targets will want; if some C and C++ interfaces to 
> half precision are standardised in future it seems likely they'd avoid 
> those restrictions; likewise the lack of _Complex __fp16.  (Indeed the ARM 
> EABI defines how half-precision arguments and return types should work at 
> the language-independent ABI level; it's just the C and C++ bindings that 
> don't allow them.)  If there is future standardisation it would seem 
> reasonable for the ARM implementation to align with it, possibly removing 
> some hooks.

Thanks for the explanation.  Then my only objections is on
convert_to_type and promoted_type, I'm convinced that the others are
indeed best left as target hooks.

BTW, is the EABI implemented for half-precision arguments and return
types?  From the 6/8 patch it does not look like and I think we should
sorry for that in the ARM backend; even if that code is currently
unreachable it may not be so in the future, because after Martin's
IPA-SRA pass goes in the middle-end might decide to have a function with
HFmode parameters or return types.

Paolo

Re: [PATCH] ARM half-precision floating point, 3/8 (target hooks)

[Joseph S. Myers]

On Thu, 16 Apr 2009, Paolo Bonzini wrote:

> 
> > The function argument and return type restrictions, and conversions from 
> > double to HFmode going via float (with double rounding), are not things 
> > that it's clear other targets will want; if some C and C++ interfaces to 
> > half precision are standardised in future it seems likely they'd avoid 
> > those restrictions; likewise the lack of _Complex __fp16.  (Indeed the ARM 
> > EABI defines how half-precision arguments and return types should work at 
> > the language-independent ABI level; it's just the C and C++ bindings that 
> > don't allow them.)  If there is future standardisation it would seem 
> > reasonable for the ARM implementation to align with it, possibly removing 
> > some hooks.
> 
> Thanks for the explanation.  Then my only objections is on
> convert_to_type and promoted_type, I'm convinced that the others are
> indeed best left as target hooks.

The convert_to_type hook implements the peculiarity of the ARM C binding 
that conversions from double to __fp16 go via float, and so is needed to 
ensure that the same double rounding applies whether or not a conversion 
is folded at compile time.  (Given the hook inserting intermediate 
conversions, the compiler itself should only be generating such 
conversions if it knows that the double rounding cannot occur, or if 
-funsafe-math-optimizations, so it should be OK for the intermediate 
conversions to be inserted again whenever such conversions are seen by the 
back end at a later stage.)

> BTW, is the EABI implemented for half-precision arguments and return
> types?  From the 6/8 patch it does not look like and I think we should
> sorry for that in the ARM backend; even if that code is currently
> unreachable it may not be so in the future, because after Martin's
> IPA-SRA pass goes in the middle-end might decide to have a function with
> HFmode parameters or return types.

A sorry sounds like it would be a good idea - care would be needed to 
ensure that whatever compiler-generated function calls it affects, it 
doesn't affect the libcalls converting between HFmode and SFmode (which 
are defined using unsigned short, which corresponds to how the compiler 
generates code using those libcalls).  Possibly the hooks determining 
types that cannot be used in calls/returns should be used by any 
optimization pass that might generate such functions?

AAPCS defines HFmode parameters and return types to work by converting the 
values to SFmode, so implementing the AAPCS for such functions (if in 
future GCC allows them to be defined) would require calls to the 
conversion libcalls to be generated as part of the function calling 
sequence.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] ARM half-precision floating point, 3/8 (target hooks)

[Paolo Bonzini]

Okay, the hooks matter is settled. :-)

Thanks for leading me through it.  The comment on double rounding should
be in arm.c, it is not clear otherwise and it would help if at any later
date other targets want to follow the same track.

The rest of the message is about the ABI issue.

> A sorry sounds like it would be a good idea [...]
> AAPCS defines HFmode parameters and return types to work by converting the 
> values to SFmode, so implementing the AAPCS for such functions (if in
> future GCC allows them to be defined) would require calls to the 
> conversion libcalls to be generated as part of the function calling 
> sequence.

And maybe the best thing is to implement it (if not now, just make sure
it gets into 4.5).  There is already a mechanism to do so,
TARGET_PROMOTE_PROTOTYPES.

The best way to do so would be IMO to extend TARGET_PROMOTE_PROTOTYPES
to get a function and a type, and return another type.  Then in the
front-end you apply the function unconditionally like

  if (TREE_CODE (decl) == FUNCTION_DECL)
    {
      struct c_declarator *ce = declarator;

      if (ce->kind == cdk_pointer)
        ce = declarator->declarator;
      if (ce->kind == cdk_function)
        {
          tree args = ce->u.arg_info->parms;
          for (; args; args = TREE_CHAIN (args))
            DECL_ARG_TYPE (args) =
              targetm.calls.promote_prototypes (decl, TREE_TYPE (args));
        }
    }

and likewise in a couple of other places (in one place
default_conversion is called instead, but you can call convert instead
with the result of the hook).

Only ARM and two other targets implement it (SH and SPARC; m32c could
use the default), and the others just use a "stock" function, so
preparing the patch is feasible.

Doing this in the front-end also fixes the problem that...

>  care would be needed to
>  ensure that whatever compiler-generated function calls it affects, it
>  doesn't affect the libcalls converting between HFmode and SFmode

Paolo

Re: [PATCH] ARM half-precision floating point, 3/8 (target hooks)

[Joseph S. Myers]

On Thu, 16 Apr 2009, Paolo Bonzini wrote:

> Doing this in the front-end also fixes the problem that...
> 
> >  care would be needed to
> >  ensure that whatever compiler-generated function calls it affects, it
> >  doesn't affect the libcalls converting between HFmode and SFmode

It may fix problems, but I think we should move away from using this hook 
in the front ends at all.  Variation between the types used in language 
terms and the final ABI should be handled at some point after the front 
ends have finished; maybe a lowering stage carried out on GIMPLE, but if 
the language allows arguments and return values of a given type I think 
the GIMPLE coming out of the front end should represent that they are of 
that type.  Note that in AAPCS the conversion from half-precision to 
single-precision is part of the language-independent argument-passing 
rules, not part of how C language types are mapped to language-independent 
types.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] ARM half-precision floating point, 3/8 (target hooks)

[Paolo Bonzini]

> It may fix problems, but I think we should move away from using this hook 
> in the front ends at all.  Variation between the types used in language 
> terms and the final ABI should be handled at some point after the front 
> ends have finished; maybe a lowering stage carried out on GIMPLE, but if 
> the language allows arguments and return values of a given type I think 
> the GIMPLE coming out of the front end should represent that they are of 
> that type.

Yes, I agree, but the libcalls are generated when we go to RTL, so the
solution I proposed would still hold if ABI handling was done on GIMPLE.

> Note that in AAPCS the conversion from half-precision to 
> single-precision is part of the language-independent argument-passing 
> rules, not part of how C language types are mapped to language-independent 
> types.

Yes, I suspected that.

Paolo

Re: [PATCH] ARM half-precision floating point, 3/8 (target hooks)

[Richard Guenther]

On Thu, Apr 16, 2009 at 4:23 PM, Paolo Bonzini <bonzini@gnu.org> wrote:
>
>> It may fix problems, but I think we should move away from using this hook
>> in the front ends at all.  Variation between the types used in language
>> terms and the final ABI should be handled at some point after the front
>> ends have finished; maybe a lowering stage carried out on GIMPLE,

I'd rather do this when expanding to RTL.  Otherwise we would
need to mess with the function type signature and this will be ugly.

Richard.

Re: [PATCH] ARM half-precision floating point, 3/8 (target hooks)

[Sandra Loosemore]

Paolo Bonzini wrote:
> Okay, the hooks matter is settled. :-)
> 
> Thanks for leading me through it.  The comment on double rounding should
> be in arm.c, it is not clear otherwise and it would help if at any later
> date other targets want to follow the same track.

I've gotten somewhat lost in this discussion.  Do I need to make any changes to 
the hooks, or just add the comment on double rounding?

> The rest of the message is about the ABI issue.
> 
>> A sorry sounds like it would be a good idea [...]
>> AAPCS defines HFmode parameters and return types to work by converting the 
>> values to SFmode, so implementing the AAPCS for such functions (if in
>> future GCC allows them to be defined) would require calls to the 
>> conversion libcalls to be generated as part of the function calling 
>> sequence.
> 
> And maybe the best thing is to implement it (if not now, just make sure
> it gets into 4.5).  There is already a mechanism to do so,
> TARGET_PROMOTE_PROTOTYPES.
> 
> The best way to do so would be IMO to extend TARGET_PROMOTE_PROTOTYPES
> to get a function and a type, and return another type.  Then in the
> front-end you apply the function unconditionally like
> 
>   if (TREE_CODE (decl) == FUNCTION_DECL)
>     {
>       struct c_declarator *ce = declarator;
> 
>       if (ce->kind == cdk_pointer)
>         ce = declarator->declarator;
>       if (ce->kind == cdk_function)
>         {
>           tree args = ce->u.arg_info->parms;
>           for (; args; args = TREE_CHAIN (args))
>             DECL_ARG_TYPE (args) =
>               targetm.calls.promote_prototypes (decl, TREE_TYPE (args));
>         }
>     }
> 
> and likewise in a couple of other places (in one place
> default_conversion is called instead, but you can call convert instead
> with the result of the hook).
> 
> Only ARM and two other targets implement it (SH and SPARC; m32c could
> use the default), and the others just use a "stock" function, so
> preparing the patch is feasible.
> 
> Doing this in the front-end also fixes the problem that...
> 
>>  care would be needed to
>>  ensure that whatever compiler-generated function calls it affects, it
>>  doesn't affect the libcalls converting between HFmode and SFmode

I've gotten lost here, too -- I can't figure out whether the subsequent 
discussion is endorsing this idea, or not.  So, what do I need to do to fix the 
patch?  Add the sorry() calls?  Add this hook function?  Both?  Something else?

-Sandra the confused

Re: [PATCH] ARM half-precision floating point, 3/8 (target hooks)

[Paolo Bonzini]

> I've gotten somewhat lost in this discussion.  Do I need to make any
> changes to the hooks, or just add the comment on double rounding?

As far as I'm concerned, just the comment.

> I've gotten lost here, too -- I can't figure out whether the subsequent
> discussion is endorsing this idea, or not.  So, what do I need to do to
> fix the patch?  Add the sorry() calls?  Add this hook function?  Both? 
> Something else?

I'm not a maintainer again, but I think that for now filing a bug is
enough.  I took a look, and I do not suggest that you lose your time
trying to prepare a patch adding a sorry() call, when maybe Joseph's
concern (that it would not work for the f2h libcalls) would make the
effort useless.

By the time stage3 is reached, it will be easier to assess if Martin's
IPA-SRA has the possibility of being broken by __fp16, and how
likely/serious that is.

> -Sandra the confused

Probably my fault, sorry.

Paolo

Re: [PATCH] ARM half-precision floating point, 3/8 (target hooks)

[Paolo Bonzini]

> The convert_to_type hook implements the peculiarity of the ARM C binding 
> that conversions from double to __fp16 go via float, and so is needed to 
> ensure that the same double rounding applies whether or not a conversion 
> is folded at compile time.

For the record, I checked what the OpenCL standard says about
half-precision floating-points and the double rounding applies there
too, as does the forbidding of arguments and return types.

The default conversion does not apply, because it is not even legal to
dereference a pointer to half-precision floating-point values.

This is not implying any judgement on the opportunity to implement the
target hooks or not; OpenCL defines a different dialect of C altogether,
 so I'm now convinced that target hooks should stay until more targets
implement the half-precision floats.

Paolo

Re: [PATCH] ARM half-precision floating point, 3/8 (target hooks)

[Richard Earnshaw]

On Thu, 2009-04-16 at 14:13 +0000, Joseph S. Myers wrote:
> Note that in AAPCS the conversion from half-precision to 
> single-precision is part of the language-independent argument-passing 
> rules, not part of how C language types are mapped to language-independent 
> types.
> 

True, but language-dependent rules would be applied first, so if, for
example, they changed an HFmode object into a DFmode object, then the
language independent rules would never see an HFmode object.

There's an interesting issue as to whether a HFmode value passed to a
variadic routine should be converted directly to DFmode or converted
first to SFmode and then to DFmode (not much of a problem at the moment
since passing HFmode objects by value isn't allowed at the moment, and
in any case there's no rounding in this direction); but there are
potential issues with the reverse conversion because of double rounding.

R.

Re: [PATCH] ARM half-precision floating point, 3/8 (target hooks)

[Paolo Bonzini]

> True, but language-dependent rules would be applied first, so if, for
> example, they changed an HFmode object into a DFmode object, then the
> language independent rules would never see an HFmode object.

Yes, so PROMOTE_FUNCTION_MODE would be the right way to implement the
AAPCS.  (The macro however does not support it yet; see my hookization
patches, after which it becomes a target hook and it is powerful enough
to distinguish conversion libcalls from other uses).

> There's an interesting issue as to whether a HFmode value passed to a
> variadic routine should be converted directly to DFmode or converted
> first to SFmode and then to DFmode (not much of a problem at the moment
> since passing HFmode objects by value isn't allowed at the moment, and
> in any case there's no rounding in this direction); but there are
> potential issues with the reverse conversion because of double rounding.

Interesting.  Indeed, as implemented now (and without the hooks
forbidding HFmode pass-by-value) Sandra's patches would perform the
double rounding since float is never passed through "...".

Paolo

[PING] Re: [PATCH] ARM half-precision floating point, 3/8 (target hooks)

[Sandra Loosemore]

Ping!  Can somebody review this patch?  There was quite a bit of discussion 
about whether this was the right approach when I posted it last month, and I 
think the consensus was that this is OK -- at least, I didn't seem to get any 
specific suggestions for things that need fixing.  But, the patch hasn't been 
formally approved by a maintainer.

http://gcc.gnu.org/ml/gcc-patches/2009-04/msg01124.html

I'll be posting revised versions of some of the later pieces of the series to 
address the comments I got on those.

Thanks,

-Sandra

> 2009-04-15  Sandra Loosemore  <sandra@codesourcery.com>
> 
>     gcc/
>     * doc/tm.texi (Misc): Document TARGET_INVALID_PARAMETER_TYPE,
>     TARGET_INVALID_RETURN_TYPE, TARGET_PROMOTED_TYPE, and
>     TARGET_CONVERT_TO_TYPE.
>     * hooks.c (hook_tree_const_tree_null): Define.
>     * hooks.h (hook_tree_const_tree_null): Declare.
>     * target.h (struct gcc_target):  Add invalid_parameter_type,
>     invalid_return_type, promoted_type, and convert_to_type fields.
>     * target-def.h: (TARGET_INVALID_PARAMETER_TYPE): Define.
>     (TARGET_INVALID_RETURN_TYPE): Define.
>     (TARGET_PROMOTED_TYPE): Define.
>     (TARGET_CONVERT_TO_TYPE): Define.
>     (TARGET_INITIALIZER): Update for new fields.
>     * c-decl.c (grokdeclarator): Check targetm.invalid_return_type.
>     (grokparms): Check targetm.invalid_parameter_type.
>     * c-typeck.c (default_conversion): Check targetm.promoted_type.
>     * c-convert.c (convert): Check targetm.convert_to_type.
>     * cp/typeck.c (default_conversion): Check targetm.promoted_type.
>     * cp/decl.c (grokdeclarator): Check targetm.invalid_return_type.
>     (grokparms): Check targetm.invalid_parameter_type.
>     * cp/cvt.c (ocp_convert): Check targetm.convert_to_type.
>     (build_expr_type_conversion): Check targetm.promoted_type.

Re: [PATCH] ARM half-precision floating point, 3/8 (target hooks)

[Ian Lance Taylor]

Sandra Loosemore <sandra@codesourcery.com> writes:

> 2009-04-15  Sandra Loosemore  <sandra@codesourcery.com>
>
> 	gcc/
> 	* doc/tm.texi (Misc): Document TARGET_INVALID_PARAMETER_TYPE,
> 	TARGET_INVALID_RETURN_TYPE, TARGET_PROMOTED_TYPE, and
> 	TARGET_CONVERT_TO_TYPE.
> 	* hooks.c (hook_tree_const_tree_null): Define.
> 	* hooks.h (hook_tree_const_tree_null): Declare.
> 	* target.h (struct gcc_target):  Add invalid_parameter_type,
> 	invalid_return_type, promoted_type, and convert_to_type fields.
> 	* target-def.h: (TARGET_INVALID_PARAMETER_TYPE): Define.
> 	(TARGET_INVALID_RETURN_TYPE): Define.
> 	(TARGET_PROMOTED_TYPE): Define.
> 	(TARGET_CONVERT_TO_TYPE): Define.
> 	(TARGET_INITIALIZER): Update for new fields.
> 	* c-decl.c (grokdeclarator): Check targetm.invalid_return_type.
> 	(grokparms): Check targetm.invalid_parameter_type.
> 	* c-typeck.c (default_conversion): Check targetm.promoted_type.
> 	* c-convert.c (convert): Check targetm.convert_to_type.
> 	* cp/typeck.c (default_conversion): Check targetm.promoted_type.
> 	* cp/decl.c (grokdeclarator): Check targetm.invalid_return_type.
> 	(grokparms): Check targetm.invalid_parameter_type.
> 	* cp/cvt.c (ocp_convert): Check targetm.convert_to_type.
> 	(build_expr_type_conversion): Check targetm.promoted_type.



> +@deftypefn {Target Hook} {const char *} TARGET_INVALID_PARAMETER_TYPE (tree @var{type})
> +If defined, this macro returns the diagnostic message when it is
> +invalid for functions to include parameters of type @var{type}, 
> +or @code{NULL} if validity should be determined by
> +the front end.
> +@end deftypefn

As far as I can tell, you have only implemented for the C and C++
frontends.  Unless you intend to extend the work other frontends, I
think that should be documented here.

> +@deftypefn {Target Hook} {const char *} TARGET_INVALID_RETURN_TYPE (tree @var{type})
> +If defined, this macro returns the diagnostic message when it is
> +invalid for functions to have return type @var{type}, 
> +or @code{NULL} if validity should be determined by
> +the front end.
> +@end deftypefn

Same comment.

> +@deftypefn {Target Hook} {tree} TARGET_PROMOTED_TYPE (tree @var{type})
> +If defined, this target hook returns the type to which values of 
> +@var{type} should be promoted when they appear in expressions, 
> +analogous to the integer promotions, or @code{NULL_TREE} to use the
> +front end's normal promotion rules.  This hook is useful when there are
> +target-specific types with special promotion rules.
> +@end deftypefn

This target hook will presumably never be used by frontends other than
C/C++, and that should be documented hre.

> +@deftypefn {Target Hook} {tree} TARGET_CONVERT_TO_TYPE (tree @var{type}, tree @var{expr})
> +If defined, this hook returns the result of converting @var{expr} to 
> +@var{type}.  It should return the converted expression, 
> +or @code{NULL_TREE} to apply the front end's normal conversion rules.
> +This hook is useful when there are target-specific types with special 
> +conversion rules.
> +@end deftypefn

I don't know whether this one is C/C++ specific or not.  I suspect that
it is.


> +  if ((e1 = targetm.convert_to_type (type, e)))
> +    return e1;

Don't do this kind of thing when you don't need to.  Replace with
something like:
  e1 = targetm.convert_to_type (type, e);
  if (e1)
    return e1;

> +  if ((ret =  targetm.convert_to_type (type, expr)))
> +      return ret;

Same here.


OK with those changes.

Thanks.

Ian