More general patch for "mips offs > 16bit",(emit_insn/force_operand inconsistencies, expr.c 1.76).

[Hans-Peter Nilsson]

We have a communications failure, yet it seems we agree it
is incorrect to pass "(plus:SI (reg:SI 22) (const_int 276004))"
to emit_move_insn, or its immediate caller force_reg (which
calls emit_move_insn unconditionally at the beginning).

Still, that (plus...) is the inner rtx of the correct
return-value from the first call to change_address, and is
passed on to force_reg.

Let's try yet a different angle for the presentation of why this
bug is not a mips port bug (only), and hopefully why it should
not be put on hold until someone fixes the mips port.
 (If it had just been a mips bug, I wouldn't really have
bothered this much, sorry).

I present a gdb session of
<URL: http://egcs.cygnus.com/ml/egcs-bugs/1999-02/msg00786.html >
on my i686-pc-linux-gnulibc1 egcs-1.1.2

In this session, I will show the (plus...) incorrectly being
passed on to emit_move_insn, from the line I believe is buggy
(last line in patch snippet below).

I set an initial breakpoint at the marked line below (note that
the line-numbers in the patch are since long off):

 *** expr.c	1998/06/24 14:49:45	1.75
 --- expr.c	1998/06/24 22:40:29	1.76
 *************** expand_assignment (to, from, want_value,
 *** 2920,2925 ****
 --- 2920,2945 ----
   #endif
             }
   
 + 	  if (GET_CODE (to_rtx) == MEM
 + 	      && GET_MODE (to_rtx) == BLKmode
 + 	      && bitsize
 + 	      && (bitpos % bitsize) == 0 
 + 	      && (bitsize % GET_MODE_ALIGNMENT (mode1)) == 0
 + 	      && (alignment * BITS_PER_UNIT) == GET_MODE_ALIGNMENT (mode1))
 + 	    {
 + 	      rtx temp = change_address (to_rtx, mode1,
We start here^
 + 				         plus_constant (XEXP (to_rtx, 0),
 + 						        (bitpos /
 + 						         BITS_PER_UNIT)));
 + 	      if (GET_CODE (XEXP (temp, 0)) == REG)
 + 	        to_rtx = temp;
 + 	      else
 + 		to_rtx = change_address (to_rtx, mode1,
 + 				         force_reg (GET_MODE (XEXP (temp, 0)),
 + 						    XEXP (temp, 0)));

Note (mostly to others):  I use shortforms of gdb commands.  I use the gdb
macros present in .gdbinit, which is put in the same directory as cc1
by the configuration process.  Comments are inserted on lines starting
with ###.  The session is run from inside emacs; you may note
discrepancies in the output compared to a free-running gdb.

...
GDB 4.16 (i586-unknown-linux), Copyright 1996 Free Software Foundation, Inc...
Breakpoint 1 at 0x8048e58
Breakpoint 2 at 0x8049038
Breakpoint 3 at 0x806f62f: file /home/hp/egcs/cvs_1.1/egcs/gcc/toplev.c, line 2121.
(gdb) b expr.c:3146
Breakpoint 4 at 0x80a6440: file /home/hp/egcs/cvs_1.1/egcs/gcc/expr.c, line 3146.
### Bug-tripping code in /tmp/p.i, as of the URL above.
(gdb) r </tmp/p.i
Starting program: /mnt/s0d6p1/hp/egcs_1_1_test/gcc/cc1 </tmp/p.i
warning: Unable to find dynamic linker breakpoint function.
warning: GDB will be unable to debug shared library initializers
warning: and track explicitly loaded dynamic code.
Breakpoint 1 at 0x40022ba4
Breakpoint 2 at 0x40031088
	.file	"stdin"
	.version	"01.01"
gcc2_compiled.:
 zwrite
Breakpoint 4, expand_assignment (to=0x8231ec0, from=0x8231f48, want_value=0, 
    suggest_reg=1) at /home/hp/egcs/cvs_1.1/egcs/gcc/expr.c:3146
### Just the incoming values that are of (some) interest.  Hope I
### did not miss any:
(gdb) p to_rtx
$1 = (struct rtx_def *) 0x82336dc
(gdb) pr
(mem/s:BLK (reg:SI 22))
(gdb) p bitsize
$2 = 16
(gdb) p bitpos
$3 = 2208032
(gdb) p mode1
$4 = HImode
(gdb) n
### Ok, after that "next", here is the result, which seems correct:
(gdb) p temp
$5 = (struct rtx_def *) 0x823374c
(gdb) pr

(mem/s:HI (plus:SI (reg:SI 22)
        (const_int 276004)))
### Let's pass the "if" and land before executing the "else" arm:
(gdb) n
### Step into force_reg:
(gdb) s
force_reg (mode=SImode, x=0x8233740)
    at /home/hp/egcs/cvs_1.1/egcs/gcc/explow.c:671
### Now, what do we have here:
(gdb) p x
$6 = (struct rtx_def *) 0x8233740
(gdb) pr

(plus:SI (reg:SI 22)
    (const_int 276004))
### Oopsi *that is not a move-insn operand*.  Let's continue
### (unconditionally) into emit_move_insn:
(gdb) n
(gdb) n
### A temporary reg is generated here (just a gen_reg_rtx), to
### be the "forced" reg:
(gdb) p temp
$7 = (struct rtx_def *) 0x8233758
(gdb) pr

(reg:SI 24)
(gdb) s
emit_move_insn (x=0x8233758, y=0x8233740)
    at /home/hp/egcs/cvs_1.1/egcs/gcc/expr.c:2417
### Just to check we're still talking about the same values:
(gdb) p x
$8 = (struct rtx_def *) 0x8233758
(gdb) pr

(reg:SI 24)
(gdb) p y
$9 = (struct rtx_def *) 0x8233740
(gdb) pr

(plus:SI (reg:SI 22)
    (const_int 276004))
### Thus, incorrect input to emit_move_insn.
(gdb) q

You have earlier clarified that force_reg is not supposed to
handle (plus...):es, and that change_address is correct in
giving the return value as in the session above, as long as it
is a valid address inside that valid (MEM...) operand for the
architecture in question.  Which it is for i386.

I have now shown why I believe this code is buggy, to a degree I
hope is satisfactory.  It also seems clear that this is
unrelated to the mips port, and that the patch in
<URL: http://egcs.cygnus.com/ml/egcs-patches/1999-02/msg00796.html >
is the correct solution (adding force_operand to the two buggy
lines in the two hunks of the 1.76 expr.c change).

If the reason to *not* correct this bug, is that it will paper
over a bug in address-recognition in the mips port, then I would
hope it is better to correct this more-general bug now, and
advise the mips port maintainer to get this item higher up on
his/hers TODO-list.  Else the buggy emit_move_insn operand will
one day cause an abort call for i386 or another (more) innocent
port.

This still seems so obvious to me that I'm afraid I'm missing
something (like the point ;-) and if I do miss something, please
(anybody), take the time to point it out.  It may help others
with a less or equal amount of clue.

brgds, H-P