Re: [Ping] RE: CR16 Port addition

[Richard Henderson <rth at redhat dot com>]

> +(define_constraint "M"
> +  "A unsigned and customized  4-bit immediate."
> +  (and (match_code "const_int")
> +       (match_test "(IN_RANGE_P (ival, 0, 15) && ((ival != 9) || (ival != 11)))  ")))

Logic error: x != 9 || x != 11 => true.

> +/* TARGETM Function Prototypes and forward declarations  */
> +static rtx cr16_struct_value_rtx (tree fntype ATTRIBUTE_UNUSED,
> +				  int incoming ATTRIBUTE_UNUSED);
> +static bool cr16_return_in_memory (const_tree type, const_tree fntype ATTRIBUTE_UNUSED);
> +static bool cr16_frame_pointer_required (void);
> +static bool cr16_can_eliminate (const int, const int);
> +static rtx cr16_legitimize_address (rtx, rtx, enum machine_mode);
> +static enum machine_mode cr16_unwind_word_mode (void);
> +static void cr16_conditional_register_usage (void);
> +static void cr16_override_options (void);
> +static bool cr16_class_likely_spilled_p (reg_class_t);
> +static int  cr16_return_pops_args (tree, tree, int);
> +static rtx cr16_function_arg (CUMULATIVE_ARGS *, enum machine_mode,
> +                             const_tree, bool);
> +static void cr16_function_arg_advance (CUMULATIVE_ARGS *, enum machine_mode,
> +                                      const_tree, bool);
> +static bool cr16_legitimate_constant_p   (enum machine_mode, rtx);
> +static rtx cr16_function_value (const_tree, const_tree, bool);
> +static rtx cr16_libcall_value (enum machine_mode, const_rtx);
> +static bool cr16_function_value_regno_p (const unsigned int);
> +static bool cr16_legitimate_address_p (enum machine_mode, rtx, bool);
> +static void cr16_print_operand (FILE *, rtx, int);
> +static void cr16_print_operand_address (FILE *, rtx);
> +static int cr16_address_cost (rtx, bool);
> +static int cr16_register_move_cost (enum machine_mode, reg_class_t, reg_class_t);
> +static int cr16_memory_move_cost (enum machine_mode, reg_class_t, bool);

Please delete as many of these as you can.  Since the TARGETM
structure is now at the end, I suspect you need none of them.

> +cr16_return_in_memory (const_tree type, const_tree fntype ATTRIBUTE_UNUSED)
> +{
> +  if (TYPE_MODE (type) == BLKmode)

Don't define your ABI based on modes.  Define your ABI in terms of types and sizes.
Different versions of the compiler can and will have different modes based on 
various optimizations.

> +    data_model = DM_DEFAULT;
> +
> +
> +}

Extra whitespace.  Check for this all over.

> +	case UNSPEC:
> +	  inform (input_location, "unspec!!");
> +	  switch (XINT (x, 1))
> +	    {
> +	    default:
> +	      gcc_unreachable ();
> +	    }
> +	  break;

Delete debuging statements.

> +  REG_NOTES (insn) = gen_rtx_EXPR_LIST (SImode, dwarf,
> +					REG_NOTES (insn));

Huh?  That's not a reg note.
Try something like add_reg_note (insn, kind, dwarf).
Certainly "SImode" is the wrong enum type.

> +/* Helper function for md file. This function is used to emit arithmetic 
> +   DI instructions. The argument "num" decides which instruction to be
> +   printed.  */
> +const char *
> +cr16_emit_add_sub_di (rtx *operands, int num)
...
> +const char *
> +cr16_emit_logical_di (rtx *operands, int num)

Why use 0, 1, 2 instead of the proper enum rtx_code?

> +#define FUNCTION_BOUNDARY  (!optimize_size ? BIGGEST_ALIGNMENT : BITS_PER_WORD)

This is part of the ABI, and so should not change based on optimization.
It should be the minimum necessary alignment of an instruction.

> +(define_expand "adddi3"
> +  [(set (match_operand:DI 0 "register_operand" "")
> +	(plus:DI (match_operand:DI 1 "register_operand" "")
> +		 (match_operand:DI 2 "nonmemory_operand" "")))]
> +  ""
> +  {
> +    if ((GET_CODE (operands[2]) != REG))
> +      {
> +	operands[2] = force_reg (DImode, operands[2]);
> +      }
> +  }
> +)

Still unfixed from last review.  Remove this expander and
rename "adddi3_insn" to "adddi3".  You do not need this.

Similarly for subdi3, anddi3, iordi3, 

> +  "*
> +    return cr16_emit_add_sub_di (operands, 0);
> +  "

"* is deprecated.  Use { } instead.  Many examples.

> +(define_insn "smachisi3"

This should be spelled "maddhisi4" so that it'll be used by generic code.

> +(define_insn "umachisi3"

"umaddhisi4".

> +(define_insn "indirect_jump_return"
> +  [(parallel
> +    [(set (pc)
> +	  (reg:SI RA_REGNUM))
> +   (return)])
> +  ]

Unfixed from last review.  Double parallel.

> 3c3
> <    2008, 2009, 2010  Free Software Foundation, Inc.
> ---
>>    2008, 2009, 2010, 2011  Free Software Foundation, Inc.
> 39a40,43

Non-context/unidiffs will not be reviewed.


r~