[PATCH] Fix PR18251: avr reload failure

[Roger Sayle]

Hi Andy,

On Mon, 14 Feb 2005, Andy Hutchinson wrote:
>
>     PR target/18251
>     * avr.md movmemhi: rewritten as RTL loop
>     (*movmemqi_insn):deleted
>     (*movmemhi):deleted
>

This patch appears to be corrupted.  Could you repost an updated patch
with the corrections below, perhaps as an attachment?


There are a number of minor of technical problems.

> /*if loop count is constant, try to use QImode counter */

Comments must have a single space separating the "/*" from the capital
letter than starts the sentence.  Comments must also end with a period,
followed by two spaces before the "*/".


> operands[2] = copy_to_mode_reg (mode,gen_int_mode (INTVAL (operands[2]), mode));

The must be a space after a comma, and no line should be longer than about
78 characters.  i.e. the line should be split something like:

  operands[2] = copy_to_mode_reg (mode, gen_int_mode (INTVAL (operands[2]),
                                                      mode));


>  (define_expand "movmemhi"
>    [(parallel [(set (match_operand:BLK 0 "memory_operand" "")
>         (match_operand:BLK 1 "memory_operand" ""))
>         (use (match_operand:HI 2 "const_int_operand" ""))
>         (use (match_operand:HI 3 "const_int_operand" ""))
> ! ])]
>    ""

By convention, the "])]" in a define_expand appears at the end
of the previous line.  Not on a line by itself.


> !     /* work out branch probability for latter use */
> !     prob=REG_BR_PROB_BASE-REG_BR_PROB_BASE/INTVAL (operands[2]);

To be paranoid, you might want to guard against INTVAL (operands[2]) == 0
to avoid a division by zero, should this ever be encountered.

You also need a space, before and after every binary operator in
an arithmetic expression, incluing the "=" in an assignment statement.


>   emit_move_insn (addr1, gen_rtx_PLUS(Pmode,addr1,const1_rtx));
>   emit_move_insn (gen_rtx_MEM(QImode,addr0),tmp_reg_rtx   );

You need a space after a function's name and before the "(" in function
calls, i.e. "gen_rtx_MEM (QImode, addr0)".  Once again spaces after commas
and no extra spaces between "tmp_reg_rtx  );".


Its probably clearer to use a new "rtx loop_reg;" to hold the loop
counter instead of reusing operands[2] continually to represent the
loop counter in mode "mode".


The concept of the patch looks good.

Many thanks in advance,

Roger
--