Re: [PATCH] New port for CR16C and CRX processors (gcc)

[Kazu Hirata <kazu at cs dot umass dot edu>]

Hi Tal,

> 2004-02-11 Tal Agmon <Tal dot Agmon at nsc dot com>
>                       Tomer Levi <Tomer dot Levi at nsc dot com>
> 
>                * Configure.in: Handle crx-*-* and cr16c-*-*.
>                * config.gcc: Handle crx-*-* and cr16c-*-*.
>                * config/crx/crx.c: new file.
>                * config/crx/crx.h: Likewise.
>                * config/crx/crx.md: Likewise.
>                * config/crx/crx-protos.h: Likewise.
>                * config/crx/t-crx: Likewise.
>                * config/cr16c/cr16c.c: Likewise.
>                * config/cr16c/cr16c.h: Likewise.
>                * config/cr16c/cr16c.md: Likewise.
>                * config/cr16c/cr16c-protos.h: Likewise.
>                * config/cr16c/t-cr16c: Likewise.

Thanks for contributing new ports.  Here are some issues I noticed.

Administrative
--------------

o You need to have a copyright assignment filed.  I am no expert on
  this, so I let other people comment on this.

Technical
---------

o Must be buildable with the latest mainline of GCC.  For example, I
  see some old constructs like gen_rtx().  I just removed the support
  for that.  You need to use gen_rtx_REG and its friends.

o Use of MODE_CC.  We are trying to move away from cc0, which is a
  special register that we used to support to represent conditon
  codes.  It would be nice (required?) if you could use a register (as
  in (reg:...)) to represent condition codes.

o Use of peephole2.  Again, we are trying to move away from
  define_peephole.  Please consider using define_peephole2 instead.

o Use of ISO C90.  We don't use PARAMS anymore as we now require an
  ISO C90-compliant compiler.  You can just write:

    extern void optimization_options (int, int);

o Target-independent comments.  We have decided not to duplicate the
  macro descriptions in tm.texi in a target header.  Here is an
  example.

  /* Define this macro to the minimum alignment enforced by hardware for the 
     stack pointer on this machine. The definition is a C expression for the 
     desired alignment (measured in bits). This value is used as a default if 
     PREFERRED_STACK_BOUNDARY is not defined. On most machines, this should be 
     the same as PARM_BOUNDARY. */

  #define STACK_BOUNDARY 32

  If you have a comment describing something unique about your
  architecture, that should of course stay there.

o I am not sure if you should directly include header files from the C
  library, which would sort of bypass the configure's benefit.  I am
  no expert on this, so I let other people comment on this.

  #include <stdio.h>
  #include <sys/time.h>

o Checks for SUBREG.  In the following functions, you might want to
  strip SUBREG and check that OP is a REG before using REGNO because
  register_operand() accepts REG as well as SUBREG.  You can catch
  these things by configuring GCC with --enable-checking=rtl.

  register_no_sp_operand (rtx op, enum machine_mode mode)
  sp_register_operand (rtx op, enum machine_mode mode)
  ra_register_operand (rtx op, enum machine_mode mode)

o The following is not a good idea (although I understand the
  temptation).  We keep a unique copy of CONST_INT for each value (see
  gen_rtx_CONST_INT() in emit-rtl.c), and we treat those copies as if
  they were declared with "const".  Perhaps you want to have a helper
  function that is called by both short_neg_immediate_p() and
  short_immediate_p().

  From short_neg_immediate_p():

    INTVAL(op) = -save_intval;
    result = short_immediate_p(op, mode);
    INTVAL(op) = save_intval;

  From medium_neg_immediate_p():

    INTVAL(op) = -save_intval;
    result = medium_immediate_p(op, mode);
    INTVAL(op) = save_intval;

Please feel free to ask any questions.

p.s.
Unfortunately, there is no single web page concisely listing
deprecated internal features of GCC.  One that comes closest is the
"Port cleanliness" section at:

  http://gcc.gnu.org/projects/beginner.html

Kazu Hirata