On Mon, 6 May 2013, DJ Delorie wrote:

> Note that I had to make a few changes (fixes?)  in the MI portions of
> gcc to avoid problems I encountered, I don't know if these changes are
> "correct" or if there are better ways to avoid those cases.  Those

In any case, they should best be posted in separate messages, each one 
with its own rationale.

> +@item -mlarge
> +@opindex mlarge
> +Use large-model addressing (20-bit pointers, 32-bit size_t).
> +
> +@item -msmall
> +@opindex msmall
> +Use small-model addressing (16-bit pointers, 16-bit size_t).

@code{size_t}.

> Index: gcc/config/msp430/predicates.md
> ===================================================================
> --- gcc/config/msp430/predicates.md   (revision 0)
> +++ gcc/config/msp430/predicates.md   (revision 0)
> @@ -0,0 +1,60 @@

This file should have copyright / license notices.

> +
> +void
> +msp430_expand_helper (rtx *operands, const char *helper_name, bool 
> const_variants)

In general each function should have a comment explaining parameters or 
return values, or saying it implements a particular standard hook (I think 
the approach elsewhere in this file of defining the hook macro immediately 
above the function, instead of having a comment, is fine, but various 
functions still need comments).

> +  if (const_variants
> +      && CONST_INT_P (operands[2])
> +      && INTVAL (operands[2]) >= 1
> +      && INTVAL (operands[2]) <= 15)
> +    {
> +      helper_const = (char *) xmalloc (strlen (helper_name) + 4);
> +      sprintf (helper_const, "%s_%ld", helper_name, INTVAL (operands[2]));

I think snprintf should be used in preference to snprintf.  And INTVAL 
returns HOST_WIDE_INT, not "long", so use HOST_WIDE_INT_PRINT_DEC instead 
of %ld (there are a couple of places in msp430_print_operand that are also 
using %ld with INTVAL).

> +  if (!rv)
> +    {
> +      fprintf (stderr, "Can't subreg!!!\n");
> +      debug_rtx (r);
> +      gcc_unreachable ();

fprintf to stderr isn't a good way of reporting internal errors.  
fatal_insn seems more like what's wanted, although this isn't actually an 
insn you have here.

> +#define TARGET_CPU_CPP_BUILTINS()               \
> +  do                                            \
> +    {                                           \
> +      builtin_define ("NO_TRAMPOLINES");        \

Don't define NO_TRAMPOLINES; that's in the user's namespace.  (For 
testcases testing NO_TRAMPOLINES, lib/gcc.exp defines it if the board file 
defines gcc,no_trampolines.)

-- 
Joseph S. Myers
jos...@codesourcery.com

Reply via email to