Ping^4 Re: Target header etc. cleanup patch

Ping^4.  Parts of this patch 
<http://gcc.gnu.org/ml/gcc-patches/2011-04/msg00130.html> are still 
pending review, for targets fr30, m32r, mn10300, rx, v850 (target 
maintainers CC:ed).  This version applies cleanly to current trunk.

I have seen no comments from the mn10300 maintainers.  For the other 
targets, Nick wanted to keep abbreviations for certain types 
<http://gcc.gnu.org/ml/gcc-patches/2011-04/msg02047.html> and no-one else 
has commented on that issue, though as I noted in 
<http://gcc.gnu.org/ml/gcc-patches/2011-04/msg02048.html> I think 
abbreviating const char * is actively bad.  Cstar - defined for mn10300, 
whose maintainers haven't commented - is also the macro I care most about 
getting rid of in what's left of this patch, as it's the one that shows up 
as a false positive in my target macro listing script (the point of the 
original patch was to get rid of several such false positives, plus making 
related cleanups that showed up in the process).

2011-05-02  Joseph Myers  <joseph@codesourcery.com>

	* config/fr30/fr30-protos.h (Mmode): Don't define.
	* config/m32r/m32r-protos.h (Mmode): Don't define.  Expand
	definition where used.
	* config/mn10300/mn10300-protos.h (Mmode, Cstar, Rclas): Don't
	define.  Expand definitions where used.
	* config/rx/rx-protos.h (Mmode, Fargs, Rcode): Don't define.
	Expand definitions where used.
	* config/rx/rx.c (rx_is_legitimate_address, rx_function_arg_size,
	rx_function_arg, rx_function_arg_advance,
	rx_function_arg_boundary): Expand definitions of those macros.
	* config/v850/v850-protos.h (Mmode): Don't define.  Expand
	definition where used.

On Mon, May 2, 2011 at 5:21 PM, Joseph S. Myers <joseph@codesourcery.com> wrote:
> Ping^4.  Parts of this patch
> <http://gcc.gnu.org/ml/gcc-patches/2011-04/msg00130.html> are still
> pending review, for targets fr30, m32r, mn10300, rx, v850 (target
> maintainers CC:ed).  This version applies cleanly to current trunk.
>
> I have seen no comments from the mn10300 maintainers.  For the other
> targets, Nick wanted to keep abbreviations for certain types
> <http://gcc.gnu.org/ml/gcc-patches/2011-04/msg02047.html> and no-one else
> has commented on that issue, though as I noted in
> <http://gcc.gnu.org/ml/gcc-patches/2011-04/msg02048.html> I think
> abbreviating const char * is actively bad.  Cstar - defined for mn10300,
> whose maintainers haven't commented - is also the macro I care most about
> getting rid of in what's left of this patch, as it's the one that shows up
> as a false positive in my target macro listing script (the point of the
> original patch was to get rid of several such false positives, plus making
> related cleanups that showed up in the process).

Ok except for the changes the target maintainer objects to.  I do agree
with Joseph and if another maintainer also says Ok I think we should
overrule him.

Thanks,
Richard.

> 2011-05-02  Joseph Myers  <joseph@codesourcery.com>
>
>        * config/fr30/fr30-protos.h (Mmode): Don't define.
>        * config/m32r/m32r-protos.h (Mmode): Don't define.  Expand
>        definition where used.
>        * config/mn10300/mn10300-protos.h (Mmode, Cstar, Rclas): Don't
>        define.  Expand definitions where used.
>        * config/rx/rx-protos.h (Mmode, Fargs, Rcode): Don't define.
>        Expand definitions where used.
>        * config/rx/rx.c (rx_is_legitimate_address, rx_function_arg_size,
>        rx_function_arg, rx_function_arg_advance,
>        rx_function_arg_boundary): Expand definitions of those macros.
>        * config/v850/v850-protos.h (Mmode): Don't define.  Expand
>        definition where used.
>
> Index: gcc/config/m32r/m32r-protos.h
> ===================================================================
> --- gcc/config/m32r/m32r-protos.h       (revision 173254)
> +++ gcc/config/m32r/m32r-protos.h       (working copy)
> @@ -21,7 +21,6 @@
>
>  /* Function prototypes that cannot exist in m32r.h due to dependency
>    complications.  */
> -#define Mmode enum machine_mode
>
>  extern void   m32r_init (void);
>  extern void   m32r_init_expanders (void);
> @@ -52,16 +51,14 @@ extern rtx    m32r_return_addr (int);
>  extern rtx    m32r_function_symbol (const char *);
>
>  #ifdef HAVE_MACHINE_MODES
> -extern int    call_operand (rtx, Mmode);
> -extern int    small_data_operand (rtx, Mmode);
> -extern int    addr24_operand (rtx, Mmode);
> -extern int    addr32_operand (rtx, Mmode);
> -extern int    call26_operand (rtx, Mmode);
> -extern int    memreg_operand (rtx, Mmode);
> -extern int    small_insn_p (rtx, Mmode);
> +extern int    call_operand (rtx, enum machine_mode);
> +extern int    small_data_operand (rtx, enum machine_mode);
> +extern int    addr24_operand (rtx, enum machine_mode);
> +extern int    addr32_operand (rtx, enum machine_mode);
> +extern int    call26_operand (rtx, enum machine_mode);
> +extern int    memreg_operand (rtx, enum machine_mode);
> +extern int    small_insn_p (rtx, enum machine_mode);
>
>  #endif /* HAVE_MACHINE_MODES */
>
>  #endif /* RTX_CODE */
> -
> -#undef  Mmode
> Index: gcc/config/rx/rx-protos.h
> ===================================================================
> --- gcc/config/rx/rx-protos.h   (revision 173254)
> +++ gcc/config/rx/rx-protos.h   (working copy)
> @@ -21,11 +21,6 @@
>  #ifndef GCC_RX_PROTOS_H
>  #define GCC_RX_PROTOS_H
>
> -/* A few abbreviations to make the prototypes shorter.  */
> -#define Mmode  enum machine_mode
> -#define Fargs  CUMULATIVE_ARGS
> -#define Rcode  enum rtx_code
> -
>  extern int             rx_align_for_label (void);
>  extern void            rx_expand_prologue (void);
>  extern int             rx_initial_elimination_offset (int, int);
> @@ -37,11 +32,13 @@ extern void             rx_emit_stack_pu
>  extern void            rx_expand_epilogue (bool);
>  extern char *          rx_gen_move_template (rtx *, bool);
>  extern bool            rx_legitimate_constant_p (enum machine_mode, rtx);
> -extern bool            rx_is_restricted_memory_address (rtx, Mmode);
> -extern bool            rx_match_ccmode (rtx, Mmode);
> +extern bool            rx_is_restricted_memory_address (rtx,
> +                                                        enum machine_mode);
> +extern bool            rx_match_ccmode (rtx, enum machine_mode);
>  extern void            rx_notice_update_cc (rtx body, rtx insn);
> -extern void            rx_split_cbranch (Mmode, Rcode, rtx, rtx, rtx);
> -extern Mmode           rx_select_cc_mode (Rcode, rtx, rtx);
> +extern void            rx_split_cbranch (enum machine_mode, enum rtx_code,
> +                                         rtx, rtx, rtx);
> +extern enum machine_mode       rx_select_cc_mode (enum rtx_code, rtx, rtx);
>  #endif
>
>  #endif /* GCC_RX_PROTOS_H */
> Index: gcc/config/rx/rx.c
> ===================================================================
> --- gcc/config/rx/rx.c  (revision 173254)
> +++ gcc/config/rx/rx.c  (working copy)
> @@ -78,7 +78,8 @@ rx_small_data_operand (rtx op)
>  }
>
>  static bool
> -rx_is_legitimate_address (Mmode mode, rtx x, bool strict ATTRIBUTE_UNUSED)
> +rx_is_legitimate_address (enum machine_mode mode, rtx x,
> +                         bool strict ATTRIBUTE_UNUSED)
>  {
>   if (RTX_OK_FOR_BASE (x, strict))
>     /* Register Indirect.  */
> @@ -809,7 +810,7 @@ rx_round_up (unsigned int value, unsigne
>    occupied by an argument of type TYPE and mode MODE.  */
>
>  static unsigned int
> -rx_function_arg_size (Mmode mode, const_tree type)
> +rx_function_arg_size (enum machine_mode mode, const_tree type)
>  {
>   unsigned int num_bytes;
>
> @@ -829,7 +830,8 @@ rx_function_arg_size (Mmode mode, const_
>    variable parameter list.  */
>
>  static rtx
> -rx_function_arg (Fargs * cum, Mmode mode, const_tree type, bool named)
> +rx_function_arg (CUMULATIVE_ARGS * cum, enum machine_mode mode,
> +                const_tree type, bool named)
>  {
>   unsigned int next_reg;
>   unsigned int bytes_so_far = *cum;
> @@ -866,14 +868,14 @@ rx_function_arg (Fargs * cum, Mmode mode
>  }
>
>  static void
> -rx_function_arg_advance (Fargs * cum, Mmode mode, const_tree type,
> -                        bool named ATTRIBUTE_UNUSED)
> +rx_function_arg_advance (CUMULATIVE_ARGS * cum, enum machine_mode mode,
> +                        const_tree type, bool named ATTRIBUTE_UNUSED)
>  {
>   *cum += rx_function_arg_size (mode, type);
>  }
>
>  static unsigned int
> -rx_function_arg_boundary (Mmode mode ATTRIBUTE_UNUSED,
> +rx_function_arg_boundary (enum machine_mode mode ATTRIBUTE_UNUSED,
>                          const_tree type ATTRIBUTE_UNUSED)
>  {
>   return 32;
> Index: gcc/config/fr30/fr30-protos.h
> ===================================================================
> --- gcc/config/fr30/fr30-protos.h       (revision 173254)
> +++ gcc/config/fr30/fr30-protos.h       (working copy)
> @@ -1,5 +1,5 @@
>  /* Prototypes for fr30.c functions used in the md file & elsewhere.
> -   Copyright (C) 1999, 2000, 2002, 2004, 2007, 2010
> +   Copyright (C) 1999, 2000, 2002, 2004, 2007, 2010, 2011
>    Free Software Foundation, Inc.
>
>  This file is part of GCC.
> @@ -28,8 +28,6 @@ extern void  fr30_print_operand (FILE *,
>  extern void  fr30_print_operand_address (FILE *, rtx);
>  extern rtx   fr30_move_double (rtx *);
>  #ifdef HAVE_MACHINE_MODES
> -#define Mmode enum machine_mode
>  extern int   fr30_const_double_is_zero (rtx);
> -#undef Mmode
>  #endif /* HAVE_MACHINE_MODES */
>  #endif /* RTX_CODE */
> Index: gcc/config/mn10300/mn10300-protos.h
> ===================================================================
> --- gcc/config/mn10300/mn10300-protos.h (revision 173254)
> +++ gcc/config/mn10300/mn10300-protos.h (working copy)
> @@ -1,5 +1,5 @@
>  /* Definitions of target machine for GNU compiler. Matsushita MN10300 series
> -   Copyright (C) 2000, 2003, 2004, 2005, 2007, 2009, 2010
> +   Copyright (C) 2000, 2003, 2004, 2005, 2007, 2009, 2010, 2011
>    Free Software Foundation, Inc.
>    Contributed by Jeff Law (law@cygnus.com).
>
> @@ -19,28 +19,25 @@
>    along with GCC; see the file COPYING3.  If not see
>    <http://www.gnu.org/licenses/>.  */
>
> -#define Mmode enum machine_mode
> -#define Cstar const char *
> -#define Rclas enum reg_class
> -
>  #ifdef RTX_CODE
>  extern rtx   mn10300_legitimize_pic_address (rtx, rtx);
>  extern int   mn10300_legitimate_pic_operand_p (rtx);
> -extern rtx   mn10300_legitimize_reload_address (rtx, Mmode, int, int, int);
> +extern rtx   mn10300_legitimize_reload_address (rtx, enum machine_mode,
> +                                               int, int, int);
>  extern bool  mn10300_function_value_regno_p (const unsigned int);
>  extern int   mn10300_get_live_callee_saved_regs (void);
> -extern bool  mn10300_hard_regno_mode_ok (unsigned int, Mmode);
> -extern bool  mn10300_modes_tieable (Mmode, Mmode);
> -extern Cstar mn10300_output_add (rtx[3], bool);
> +extern bool  mn10300_hard_regno_mode_ok (unsigned int, enum machine_mode);
> +extern bool  mn10300_modes_tieable (enum machine_mode, enum machine_mode);
> +extern const char *mn10300_output_add (rtx[3], bool);
>  extern void  mn10300_print_operand (FILE *, rtx, int);
>  extern void  mn10300_print_operand_address (FILE *, rtx);
>  extern void  mn10300_print_reg_list (FILE *, int);
> -extern Mmode mn10300_select_cc_mode (enum rtx_code, rtx, rtx);
> -extern int   mn10300_store_multiple_operation (rtx, Mmode);
> -extern int   mn10300_symbolic_operand (rtx, Mmode);
> -extern void  mn10300_split_cbranch (Mmode, rtx, rtx);
> +extern enum machine_mode mn10300_select_cc_mode (enum rtx_code, rtx, rtx);
> +extern int   mn10300_store_multiple_operation (rtx, enum machine_mode);
> +extern int   mn10300_symbolic_operand (rtx, enum machine_mode);
> +extern void  mn10300_split_cbranch (enum machine_mode, rtx, rtx);
>  extern int   mn10300_split_and_operand_count (rtx);
> -extern bool  mn10300_match_ccmode (rtx, Mmode);
> +extern bool  mn10300_match_ccmode (rtx, enum machine_mode);
>  #endif /* RTX_CODE */
>
>  extern bool  mn10300_regno_in_class_p (unsigned, int, bool);
> @@ -50,7 +47,3 @@ extern void  mn10300_expand_prologue (vo
>  extern void  mn10300_expand_epilogue (void);
>  extern int   mn10300_initial_offset (int, int);
>  extern int   mn10300_frame_size (void);
> -
> -#undef Mmode
> -#undef Cstar
> -#undef Rclas
> Index: gcc/config/v850/v850-protos.h
> ===================================================================
> --- gcc/config/v850/v850-protos.h       (revision 173254)
> +++ gcc/config/v850/v850-protos.h       (working copy)
> @@ -23,8 +23,6 @@
>  #ifndef GCC_V850_PROTOS_H
>  #define GCC_V850_PROTOS_H
>
> -#define Mmode enum machine_mode
> -
>  extern void   expand_prologue               (void);
>  extern void   expand_epilogue               (void);
>  extern int    v850_handle_pragma            (int (*)(void), void (*)(int), char *);
> @@ -41,12 +39,14 @@ extern char * construct_restore_jr
>  #ifdef HAVE_MACHINE_MODES
>  extern char * construct_dispose_instruction (rtx);
>  extern char * construct_prepare_instruction (rtx);
> -extern int    ep_memory_operand             (rtx, Mmode, int);
> -extern int    v850_float_z_comparison_operator (rtx, Mmode);
> -extern int    v850_float_nz_comparison_operator (rtx, Mmode);
> -extern rtx    v850_gen_compare              (enum rtx_code, Mmode, rtx, rtx);
> -extern Mmode  v850_gen_float_compare (enum rtx_code, Mmode, rtx, rtx);
> -extern Mmode  v850_select_cc_mode (RTX_CODE, rtx, rtx);
> +extern int    ep_memory_operand             (rtx, enum machine_mode, int);
> +extern int    v850_float_z_comparison_operator (rtx, enum machine_mode);
> +extern int    v850_float_nz_comparison_operator (rtx, enum machine_mode);
> +extern rtx    v850_gen_compare              (enum rtx_code, enum machine_mode,
> +                                            rtx, rtx);
> +extern enum machine_mode  v850_gen_float_compare (enum rtx_code,
> +                                                 enum machine_mode, rtx, rtx);
> +extern enum machine_mode  v850_select_cc_mode (RTX_CODE, rtx, rtx);
>  #endif
>  #endif /* RTX_CODE */
>
> @@ -67,6 +67,4 @@ extern void ghs_pragma_endtda             (stru
>  extern void ghs_pragma_endsda              (struct cpp_reader *);
>  extern void ghs_pragma_endzda              (struct cpp_reader *);
>
> -#undef  Mmode
> -
>  #endif /* ! GCC_V850_PROTOS_H */
>
> --
> Joseph S. Myers
> joseph@codesourcery.com
>

On 05/02/2011 05:27 PM, Richard Guenther wrote:
> On Mon, May 2, 2011 at 5:21 PM, Joseph S. Myers <joseph@codesourcery.com> wrote:
>> Ping^4.  Parts of this patch
>> <http://gcc.gnu.org/ml/gcc-patches/2011-04/msg00130.html> are still
>> pending review, for targets fr30, m32r, mn10300, rx, v850 (target
>> maintainers CC:ed).  This version applies cleanly to current trunk.
>>
>> I have seen no comments from the mn10300 maintainers.  For the other
>> targets, Nick wanted to keep abbreviations for certain types
>> <http://gcc.gnu.org/ml/gcc-patches/2011-04/msg02047.html> and no-one else
>> has commented on that issue, though as I noted in
>> <http://gcc.gnu.org/ml/gcc-patches/2011-04/msg02048.html> I think
>> abbreviating const char * is actively bad.  Cstar - defined for mn10300,
>> whose maintainers haven't commented - is also the macro I care most about
>> getting rid of in what's left of this patch, as it's the one that shows up
>> as a false positive in my target macro listing script (the point of the
>> original patch was to get rid of several such false positives, plus making
>> related cleanups that showed up in the process).
> 
> Ok except for the changes the target maintainer objects to.  I do agree
> with Joseph and if another maintainer also says Ok I think we should
> overrule him.

I'll say OK then.


Bernd

Hi Joseph,

> I have seen no comments from the mn10300 maintainers.
> I think
> abbreviating const char * is actively bad.  Cstar - defined for mn10300,
> whose maintainers haven't commented

Whilst I am not a maintainer for the MN10300, I am the author of that 
bit of code.  I can say that I have no objections to removing the use of 
Cstar, it was just a syntactic shorthand, and not a very nice one at that.

If you do not get a response from Jeff or Alex you might like to ping 
rth, as he has been actively reviewing my recent MN10300 patches.

Cheers
   Nick