Re: [PATCH v5 01/10] Initial TI PRU GCC port

[Richard Sandiford <richard dot sandiford at arm dot com>]

Hi Dimitar,

Thanks for the updates, looks good.  Just a few minor things:

Dimitar Dimitrov <dimitar@dinux.eu> writes:
> +(define_predicate "pru_muldst_operand"
> +  (match_code "subreg,reg")
> +{
> +  if (register_operand (op, mode))
> +    {
> +      int regno;
> +
> +      if (REG_P (op))
> +	regno = REGNO (op);
> +      else if (GET_CODE (op) == SUBREG && REG_P (SUBREG_REG (op)))
> +	regno = REGNO (SUBREG_REG (op));
> +      else
> +	return 0;
> +
> +
> +      return REGNO_REG_CLASS (regno) == MULDST_REGS
> +	     || regno >= FIRST_PSEUDO_REGISTER;
> +    }
> +  return 0;
> +})
> +
> +(define_predicate "pru_mulsrc0_operand"
> +  (match_code "subreg,reg")
> +{
> +  if (register_operand (op, mode))
> +    {
> +      int regno;
> +
> +      if (REG_P (op))
> +	regno = REGNO (op);
> +      else if (GET_CODE (op) == SUBREG && REG_P (SUBREG_REG (op)))
> +	regno = REGNO (SUBREG_REG (op));
> +      else
> +	return 0;
> +
> +
> +      return REGNO_REG_CLASS (regno) == MULSRC0_REG
> +	     || regno >= FIRST_PSEUDO_REGISTER;
> +    }
> +  return 0;
> +})
> +
> +(define_predicate "pru_mulsrc1_operand"
> +  (match_code "subreg,reg")
> +{
> +  if (register_operand (op, mode))
> +    {
> +      int regno;
> +
> +      if (REG_P (op))
> +	regno = REGNO (op);
> +      else if (GET_CODE (op) == SUBREG && REG_P (SUBREG_REG (op)))
> +	regno = REGNO (SUBREG_REG (op));
> +      else
> +	return 0;
> +
> +
> +      return REGNO_REG_CLASS (regno) == MULSRC1_REG
> +	     || regno >= FIRST_PSEUDO_REGISTER;
> +    }
> +  return 0;
> +})

Some doubled empty lines here.

> +; Note: "JUMP_INSNs and CALL_INSNs are not allowed to have any output
> +; reloads;".  Hence this insn must be prepared for a counter that is
> +; not a register.
> +(define_insn "doloop_end_internal<mode>"
> +  [(set (pc)
> +	(if_then_else (ne (match_operand:HISI 0 "nonimmediate_operand" "+r,*m")
> +			  (const_int 1))
> +		      (label_ref (match_operand 1 "" ""))
> +		      (pc)))
> +   (set (match_dup 0)
> +	(plus:HISI (match_dup 0)
> +		 (const_int -1)))
> +   (unspec [(match_operand 2 "const_int_operand" "")] UNSPECV_LOOP_END)
> +   (clobber (match_scratch:HISI 3 "=X,&r"))]
> +  ""
> +{
> +  gcc_unreachable ();
> +}
> +  ;; Worst case length:
> +  ;;
> +  ;;	  lbbo op3_reg, op3_ptr	  4'
> +  ;;	  sub <op3_reg>, 1	  4
> +  ;;	  qbeq .+8, <op3_reg>, 0  4
> +  ;;	  jmp <op1>		  4
> +  ;;	  sbbo op3_reg, op3_ptr	  4
> +  [(set (attr "length")
> +      (if_then_else
> +	(and (ge (minus (pc) (match_dup 1)) (const_int 0))
> +	     (le (minus (pc) (match_dup 1)) (const_int 1020)))
> +	(cond [(eq_attr "alternative" "0") (const_int 4)
> +	       (eq_attr "alternative" "0") (const_int 12)]
> +	       (const_int 4))
> +	(cond [(eq_attr "alternative" "0") (const_int 12)
> +	       (eq_attr "alternative" "0") (const_int 20)]
> +	       (const_int 4))))])

The second (eq_attr "alternative" "0") lines in each (cond ...)
won't be used, since the first match wins.

> +mno-relax
> +Target Report RejectNegative
> +Make GCC pass --no-relax command-line option to the linker instead of
> +the --relax option.

A couple of missing "the"s:

  Make GCC pass the --no-relax command-line option to the linker instead
  of the --relax option

> +Link with a minimum runtime environment, with no support for static
> +initializers and constructors.  Using this option can significantly reduce
> +the size of the final ELF binary.  Beware that compiler could still generate

Sorry, this was my fault, should be:

  s/that compiler/that the compiler/

> +@item -mno-relax
> +@opindex mno-relax
> +Make GCC pass @option{--no-relax} command-line option to the linker
> +instead of the @option{--relax} option.

Missing "the" before "@option{--no-relax}"

OK with those changes once the port is accepted.  (No need to repost,
just fix up locally and commit the fixed version when the time comes.)

Jeff, could you ask the SC about accepting the port, if that hasn't
already been decided?  Dimitar, I assume you'd be OK with being the
maintainer?

Thanks,
Richard