Re: i370 port

["Ulrich Weigand" <uweigand@de.ibm.com>]

Paul Edwards wrote:
> >> Unfortunately it's not quite right, seemingly not loading R9 properly:
> >>
> >> LR    9,13
> >> AR    9,13
> >> MVC   0(10,9),0(2)
> 
> > That's weird.  What does the reload dump (.greg) say?
> 
> I have trimmed the code down to a reasonably small size so that I
> could provide the .greg file (below) from the "-da" option.  I don't
> know how to read it so I don't know if I've provided everything
> required.
> 
> Here is the current problematic generated code:
> 
> * Function pdosLoadExe code
>          L     2,4(11)
>          MVC   88(4,13),=A(LC0)
>          ST    2,92(13)
>          LA    1,88(,13)
>          L     15,=V(PRINTF)
>          BALR  14,15
>          LR    3,13           <========= probably wrong
>          AR    3,13           <========= else this is wrong
>          MVC   0(10,3),0(2)

Reload decides on the following actions:

> Reloads for insn # 38
> Reload 0: reload_in (SI) = (const_int 32880 [0x8070])
>          ADDR_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 0)
>          reload_in_reg: (const_int 32880 [0x8070])
>          reload_reg_rtx: (reg:SI 3 3)
> Reload 1: reload_in (SI) = (plus:SI (reg/f:SI 13 13)
>                                                     (const_int 32880 
> [0x8070]))
>          ADDR_REGS, RELOAD_FOR_INPUT (opnum = 0)
>          reload_in_reg: (plus:SI (reg/f:SI 13 13)
>                                                     (const_int 32880 
> [0x8070]))
>          reload_reg_rtx: (reg:SI 3 3)

That is, first: load the constant 32880 into register 3,
and second: using that reloaded constant, compute the sum
of register 13 plus 32880 and load the result also into
register 3.  Then, use that register for addressing.

This leads to the following generated code:

> (insn 271 37 273 0 (set (reg:SI 3 3)
>         (const_int 32880 [0x8070])) 15 {movsi} (nil)
>     (nil))

Load constant into register 3.

> (insn 273 271 274 0 (set (reg:SI 3 3)
>         (reg/f:SI 13 13)) 15 {movsi} (nil)
>     (nil))
> 
> (insn 274 273 38 0 (set (reg:SI 3 3)
>         (plus:SI (reg:SI 3 3)
>             (reg:SI 3 3))) 41 {addsi3} (nil)
>     (expr_list:REG_EQUIV (plus:SI (reg/f:SI 13 13)
>             (reg:SI 3 3))
>         (nil)))

Compute the sum.  Note that this code is wrong.

> (insn 38 274 41 0 (parallel [
>             (set (mem/s:BLK (reg:SI 3 3) [6 srchprog+0 S10 A64])
>                 (mem:BLK (reg/v/f:SI 2 2 [orig:27 prog ] [27]) [0 S10 A8]))
>             (use (const_int 10 [0xa]))
>         ]) 25 {*i370.md:1623} (insn_list 37 (nil))
>     (nil))

Use register 3 for adressing.

The wrong code comes in when generating the sum (insns 273/274).
I would have expected this to be a simple addsi3 instruction, along the
lines of

(set (reg:SI 3 3) (plus:SI (reg:SI 3 3) (reg:SI 13 13)))

Note that the incoming pattern:

(set (reg:SI 3 3) (plus:SI (reg:SI 13 13) (reg:SI 3 3)))

cannot be immediately resolved, since addsi3 requires the first
operand of the plus to match the result.

However, this could have been fixed by just swapping the operands.
Instead, the code attempts to create the match by reloading the
first operand (reg 13) into the output (reg 3) -- this is bogus,
since it thereby clobbers the *second* input operand, which happens
to match the output.

The code that generates these insns is in reload1.c:gen_reload

      /* We need to compute the sum of a register or a MEM and another
         register, constant, or MEM, and put it into the reload
         register.  The best possible way of doing this is if the machine
         has a three-operand ADD insn that accepts the required operands.

         The simplest approach is to try to generate such an insn and see if it
         is recognized and matches its constraints.  If so, it can be used.

         It might be better not to actually emit the insn unless it is valid,
         but we need to pass the insn as an operand to `recog' and
         `extract_insn' and it is simpler to emit and then delete the insn if
         not valid than to dummy things up.  */

      rtx op0, op1, tem, insn;
      int code;

      op0 = find_replacement (&XEXP (in, 0));
      op1 = find_replacement (&XEXP (in, 1));

      /* Since constraint checking is strict, commutativity won't be
         checked, so we need to do that here to avoid spurious failure
         if the add instruction is two-address and the second operand
         of the add is the same as the reload reg, which is frequently
         the case.  If the insn would be A = B + A, rearrange it so
         it will be A = A + B as constrain_operands expects.  */

      if (GET_CODE (XEXP (in, 1)) == REG
          && REGNO (out) == REGNO (XEXP (in, 1)))
        tem = op0, op0 = op1, op1 = tem;

      if (op0 != XEXP (in, 0) || op1 != XEXP (in, 1))
        in = gen_rtx_PLUS (GET_MODE (in), op0, op1);

      insn = emit_insn (gen_rtx_SET (VOIDmode, out, in));
      code = recog_memoized (insn);

Note how this actually performs the check whether to swap operands
for commutativity.

Can you debug this and find out why this doesn't work in your case?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com