This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix illegal cast to rtx (*insn_gen_fn) (rtx, ...)


Hi,

On Fri, 2013-07-26 at 08:51 +0200, Uros Bizjak wrote:

> BTW: I am not c++ expert, but doesn't c++ offer some sort of
> abstraction to get rid of
> 
> +  union {
> +    rtx (*argc0) (void);
> +    rtx (*argc1) (rtx);
> +    rtx (*argc2) (rtx, rtx);
> +    rtx (*argc3) (rtx, rtx, rtx);
> +    rtx (*argc4) (rtx, rtx, rtx, rtx);
> +    rtx (*argc5) (rtx, rtx, rtx, rtx, rtx);
> +    rtx (*argc6) (rtx, rtx, rtx, rtx, rtx, rtx);
> +    rtx (*argc7) (rtx, rtx, rtx, rtx, rtx, rtx, rtx);
> +    rtx (*argc8) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx);
> +    rtx (*argc9) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx);
> +    rtx (*argc10) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx);
> +    rtx (*argc11) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx);
> +  } genfun;
> 

Variadic templates maybe, but we can't use them since that requires C
++11.

Anyway, I think it's better to turn the insn_gen_fn typedef in recog.h
into a functor.  The change is quite transparent to the users, as the
attached patch shows.  There really is no need for things like GEN_FCN?,
nor do we need to fixup all the backends etc.

I've tested the attached patch on my SH cross setup with 'make all-gcc'
and it can still produce a valid 'hello world'.  Not sure whether it's
sufficient.

Some notes regarding the patch:

* The whole arg list thing could probably be folded with x-macros or
something like that, but I don't think it's worth doing it for this
single case.  If we can use C++11 in GCC at some point in time in the
future, the insn_gen_fn implementation can be folded with variadic
templates without touching all the code that uses it.

* I had to extend the number of max. args to 16, otherwise the SH
backend's sync.md code wouldn't compile.

* I don't know whether it's really needed to properly format the code of
class insn_gen_fn.  After reading the first two or three overloads
(which do fit into 80 columns) one gets the idea and so I guess nobody
is going to read that stuff completely anyway.

* The class insn_gen_fn is a POD, so it can be passed by value without
any overhead, just like a normal function pointer.  Same goes for the
function call through the wrapper class.

* Initially I had overloaded constructors in class insn_gen_fn which
worked and didn't require the cast in genoutput.c.  However, it
introduced static initializer functions and defeated the purpose of the
generated const struct insn_data_d insn_data[].  This is worked around
by casting the function pointer to insn_gen_fn::stored_funcptr. (Since
it's C++ and not C it won't implicitly cast to void*).

* The whole thing will fall apart if the stored pointer to a function
'rtx (*)(void)' is different from a stored pointer to e.g. 'rtx
(*)(rtx)'.  But I guess this is not likely to happen.

Cheers,
Oleg

gcc/ChangeLog:
	* recog.h (rtx (*insn_gen_fn) (rtx, ...)): Replace typedef with
	new class insn_gen_fn.
	* expr.c (move_by_pieces_1, store_by_pieces_2): Replace
	argument rtx (*) (rtx, ...) with insn_gen_fn.
	* genoutput.c (output_insn_data): Cast gen_? function pointers
	to insn_gen_fn::stored_funcptr.  Add initializer braces.
Index: gcc/expr.c
===================================================================
--- gcc/expr.c	(revision 201282)
+++ gcc/expr.c	(working copy)
@@ -119,7 +119,7 @@
   int reverse;
 };
 
-static void move_by_pieces_1 (rtx (*) (rtx, ...), enum machine_mode,
+static void move_by_pieces_1 (insn_gen_fn, machine_mode,
 			      struct move_by_pieces_d *);
 static bool block_move_libcall_safe_for_call_parm (void);
 static bool emit_block_move_via_movmem (rtx, rtx, rtx, unsigned, unsigned, HOST_WIDE_INT);
@@ -128,7 +128,7 @@
 static rtx clear_by_pieces_1 (void *, HOST_WIDE_INT, enum machine_mode);
 static void clear_by_pieces (rtx, unsigned HOST_WIDE_INT, unsigned int);
 static void store_by_pieces_1 (struct store_by_pieces_d *, unsigned int);
-static void store_by_pieces_2 (rtx (*) (rtx, ...), enum machine_mode,
+static void store_by_pieces_2 (insn_gen_fn, machine_mode,
 			       struct store_by_pieces_d *);
 static tree clear_storage_libcall_fn (int);
 static rtx compress_float_constant (rtx, rtx);
@@ -1043,7 +1043,7 @@
    to make a move insn for that mode.  DATA has all the other info.  */
 
 static void
-move_by_pieces_1 (rtx (*genfun) (rtx, ...), enum machine_mode mode,
+move_by_pieces_1 (insn_gen_fn genfun, machine_mode mode,
 		  struct move_by_pieces_d *data)
 {
   unsigned int size = GET_MODE_SIZE (mode);
@@ -2657,7 +2657,7 @@
    to make a move insn for that mode.  DATA has all the other info.  */
 
 static void
-store_by_pieces_2 (rtx (*genfun) (rtx, ...), enum machine_mode mode,
+store_by_pieces_2 (insn_gen_fn genfun, machine_mode mode,
 		   struct store_by_pieces_d *data)
 {
   unsigned int size = GET_MODE_SIZE (mode);
Index: gcc/genoutput.c
===================================================================
--- gcc/genoutput.c	(revision 201282)
+++ gcc/genoutput.c	(working copy)
@@ -404,9 +404,9 @@
 	}
 
       if (d->name && d->name[0] != '*')
-	printf ("    (insn_gen_fn) gen_%s,\n", d->name);
+	printf ("    { (insn_gen_fn::stored_funcptr) gen_%s },\n", d->name);
       else
-	printf ("    0,\n");
+	printf ("    { 0 },\n");
 
       printf ("    &operand_data[%d],\n", d->operand_number);
       printf ("    %d,\n", d->n_generator_args);
Index: gcc/recog.h
===================================================================
--- gcc/recog.h	(revision 201282)
+++ gcc/recog.h	(working copy)
@@ -256,8 +256,58 @@
 
 typedef int (*insn_operand_predicate_fn) (rtx, enum machine_mode);
 typedef const char * (*insn_output_fn) (rtx *, rtx);
-typedef rtx (*insn_gen_fn) (rtx, ...);
 
+struct insn_gen_fn
+{
+  typedef rtx (*f0) (void);
+  typedef rtx (*f1) (rtx);
+  typedef rtx (*f2) (rtx, rtx);
+  typedef rtx (*f3) (rtx, rtx, rtx);
+  typedef rtx (*f4) (rtx, rtx, rtx, rtx);
+  typedef rtx (*f5) (rtx, rtx, rtx, rtx, rtx);
+  typedef rtx (*f6) (rtx, rtx, rtx, rtx, rtx, rtx);
+  typedef rtx (*f7) (rtx, rtx, rtx, rtx, rtx, rtx, rtx);
+  typedef rtx (*f8) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx);
+  typedef rtx (*f9) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx);
+  typedef rtx (*f10) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx);
+  typedef rtx (*f11) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx);
+  typedef rtx (*f12) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx);
+  typedef rtx (*f13) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx);
+  typedef rtx (*f14) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx);
+  typedef rtx (*f15) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx);
+  typedef rtx (*f16) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx);
+
+  typedef f0 stored_funcptr;
+
+  rtx operator () (void) const { return ((f0)func) (); }
+  rtx operator () (rtx a0) const { return ((f1)func) (a0); }
+  rtx operator () (rtx a0, rtx a1) const { return ((f2)func) (a0, a1); }
+  rtx operator () (rtx a0, rtx a1, rtx a2) const { return ((f3)func) (a0, a1, a2); }
+  rtx operator () (rtx a0, rtx a1, rtx a2, rtx a3) const { return ((f4)func) (a0, a1, a2, a3); }
+  rtx operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4) const { return ((f5)func) (a0, a1, a2, a3, a4); }
+  rtx operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5) const { return ((f6)func) (a0, a1, a2, a3, a4, a5); }
+  rtx operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5, rtx a6) const { return ((f7)func) (a0, a1, a2, a3, a4, a5, a6); }
+  rtx operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5, rtx a6, rtx a7) const { return ((f8)func) (a0, a1, a2, a3, a4, a5, a6, a7); }
+  rtx operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5, rtx a6, rtx a7, rtx a8) const { return ((f9)func) (a0, a1, a2, a3, a4, a5, a6, a7, a8); }
+  rtx operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5, rtx a6, rtx a7, rtx a8, rtx a9) const { return ((f10)func) (a0, a1, a2, a3, a4, a5, a6, a7, a8, a9); }
+  rtx operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5, rtx a6, rtx a7, rtx a8, rtx a9, rtx a10) const { return ((f11)func) (a0, a1, a2, a3, a4, a5, a6, a7, a8, a9, a10); }
+  rtx operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5, rtx a6, rtx a7, rtx a8, rtx a9, rtx a10, rtx a11) const { return ((f12)func) (a0, a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11); }
+  rtx operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5, rtx a6, rtx a7, rtx a8, rtx a9, rtx a10, rtx a11, rtx a12) const { return ((f13)func) (a0, a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11, a12); }
+  rtx operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5, rtx a6, rtx a7, rtx a8, rtx a9, rtx a10, rtx a11, rtx a12, rtx a13) const { return ((f14)func) (a0, a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11, a12, a13); }
+  rtx operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5, rtx a6, rtx a7, rtx a8, rtx a9, rtx a10, rtx a11, rtx a12, rtx a13, rtx a14) const { return ((f15)func) (a0, a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11, a12, a13, a14); }
+  rtx operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5, rtx a6, rtx a7, rtx a8, rtx a9, rtx a10, rtx a11, rtx a12, rtx a13, rtx a14, rtx a15) const { return ((f16)func) (a0, a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11, a12, a13, a14, a15); }
+
+  // This is for compatibility of code that invokes functions like
+  //   (*funcptr) (arg)
+  insn_gen_fn operator * (void) const { return *this; }
+
+  // The wrapped function pointer must be public and there must not be any
+  // constructors.  Otherwise the insn_data_d struct initializers generated
+  // by genoutput.c will result in static initializer functions, which defeats
+  // the purpose of the generated insn_data_d array.
+  stored_funcptr func;
+};
+
 struct insn_operand_data
 {
   const insn_operand_predicate_fn predicate;

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]