Patchwork [buildbot] r201513: Invalid cast

login
register
mail settings
Submitter Oleg Endo
Date Aug. 6, 2013, 9:20 p.m.
Message ID <1375824046.3952.84.camel@yam-132-YW-E178-FTW>
Download mbox | patch
Permalink /patch/265244/
State New
Headers show

Comments

Oleg Endo - Aug. 6, 2013, 9:20 p.m.
On Tue, 2013-08-06 at 13:27 +0200, Jan-Benedict Glaw wrote:
> Hi!
> 
> On Mon, 2013-08-05 22:09:45 -0000, olegendo@gcc.gnu.org <olegendo@gcc.gnu.org> wrote:
> > Author: olegendo
> > Date: Mon Aug  5 22:09:45 2013
> > New Revision: 201513
> > 
> > URL: http://gcc.gnu.org/viewcvs?rev=201513&root=gcc&view=rev
> > Log:
> > 	PR other/12081
> > 	* 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.
> 
> This probably caused in a powerpc64-linux build:
> 
> g++ -c   -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE  -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common  -DHAVE_CONFIG_H -I. -I. -I../../../../gcc/gcc -I../../../../gcc/gcc/. -I../../../../gcc/gcc/../include -I../../../../gcc/gcc/../libcpp/include  -I../../../../gcc/gcc/../libdecnumber -I../../../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber -I../../../../gcc/gcc/../libbacktrace    \
>                 ../../../../gcc/gcc/config/rs6000/rs6000.c -o rs6000.o
> In file included from ../../../../gcc/gcc/config/rs6000/rs6000.c:36:0:
> ../../../../gcc/gcc/config/rs6000/rs6000.c: In function ‘void rs6000_emit_swdiv(rtx, rtx, rtx, bool)’:
> ../../../../gcc/gcc/optabs.h:47:46: error: invalid cast from type ‘const insn_gen_fn’ to type ‘gen_2arg_fn_t {aka rtx_def* (*)(rtx_def*, rtx_def*, rtx_def*)}’
>  #define GEN_FCN(CODE) (insn_data[CODE].genfun)
>                                               ^
> ../../../../gcc/gcc/config/rs6000/rs6000.c:28145:43: note: in expansion of macro ‘GEN_FCN’
>    gen_2arg_fn_t gen_mul = (gen_2arg_fn_t) GEN_FCN (code);
>                                            ^
> ../../../../gcc/gcc/config/rs6000/rs6000.c: In function ‘void rs6000_emit_swrsqrt(rtx, rtx)’:
> ../../../../gcc/gcc/optabs.h:47:46: error: invalid cast from type ‘const insn_gen_fn’ to type ‘gen_2arg_fn_t {aka rtx_def* (*)(rtx_def*, rtx_def*, rtx_def*)}’
>  #define GEN_FCN(CODE) (insn_data[CODE].genfun)
>                                               ^
> ../../../../gcc/gcc/config/rs6000/rs6000.c:28223:43: note: in expansion of macro ‘GEN_FCN’
>    gen_2arg_fn_t gen_mul = (gen_2arg_fn_t) GEN_FCN (code);
>                                            ^
> 
> Other PPC/RS6k targets (ie. rs6000-ibm-aix4.3, ...) seem to be equally
> affected.

That gen_2arg_fn_t typedef looks strange.  In other places where GEN_FCN
is used in similar ways in rs6000.c, there's no cast to a function
pointer with an explicit number of arguments.  It's not clear to me what
it's trying to achieve there.

I've briefly tested the attached patch by building a powerpc-elf cross
compiler with make all-gcc and checking that it can produce some
assembly code.  Is this OK for trunk?

Speaking of GEN_FCN usage in rs6000.c.  The recently added HTM builtin
code has one interesting piece:

static rtx
htm_expand_builtin (tree exp, rtx target, bool * expandedp)
{
...
	switch (nopnds)
	  {
	  case 0:
	    pat = GEN_FCN (icode) (NULL_RTX);
	    break;
	  case 1:
	    pat = GEN_FCN (icode) (op[0]);
	    break;

The 'case 0' looks suspicious.  If the function behind the pointer
really is a zero-arg function this might or might not work depending on
the ABI.  I'm not sure what the intention here is.  I've compiled
gcc/testsuite/gcc.target/powerpc/htm-builtin-1.c to see if it ever gets
into the 'case 0' but it doesn't.
The function 'htm_init_builtins' doesn't seem to handle a 'case 0'.  I'm
confused, somebody else should have a look at this please.

Cheers,
Oleg

gcc/ChangeLog:
	PR other/12081
	config/rs6000/rs6000.c (gen_2arg_fn_t): Remove typedef.
	(rs6000_emit_swdiv, rs6000_emit_swrsqrt): Don't cast result of 
	GEN_FCN to gen_2arg_fn_t.
Richard Henderson - Aug. 6, 2013, 10:50 p.m.
On 08/06/2013 11:20 AM, Oleg Endo wrote:
> 	PR other/12081
> 	config/rs6000/rs6000.c (gen_2arg_fn_t): Remove typedef.
> 	(rs6000_emit_swdiv, rs6000_emit_swrsqrt): Don't cast result of 
> 	GEN_FCN to gen_2arg_fn_t.

Ok.


r~

Patch

Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 201512)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -284,9 +284,6 @@ 
   { "rsqrtd",	 (RECIP_DF_RSQRT | RECIP_V2DF_RSQRT) },
 };
 
-/* 2 argument gen function typedef.  */
-typedef rtx (*gen_2arg_fn_t) (rtx, rtx, rtx);
-
 /* Pointer to function (in rs6000-c.c) that can define or undefine target
    macros that have changed.  Languages that don't support the preprocessor
    don't link in rs6000-c.c, so we can't call it directly.  */
@@ -28142,7 +28139,7 @@ 
     passes++;
 
   enum insn_code code = optab_handler (smul_optab, mode);
-  gen_2arg_fn_t gen_mul = (gen_2arg_fn_t) GEN_FCN (code);
+  insn_gen_fn gen_mul = GEN_FCN (code);
 
   gcc_assert (code != CODE_FOR_nothing);
 
@@ -28220,7 +28217,7 @@ 
   int i;
   rtx halfthree;
   enum insn_code code = optab_handler (smul_optab, mode);
-  gen_2arg_fn_t gen_mul = (gen_2arg_fn_t) GEN_FCN (code);
+  insn_gen_fn gen_mul = GEN_FCN (code);
 
   gcc_assert (code != CODE_FOR_nothing);