[RFA:] Fix PR 24912, broken m68k: improve gen_reload for unops.

[Hans-Peter Nilsson <hans-peter dot nilsson at axis dot com>]

Reload operated on:
(insn 17 16 18 0 (set (reg:SI 8 %a0 [35])
        (plus:SI (sign_extend:SI (reg:HI 0 %d0 [34]))
            (reg/v:SI 1 %d1 [orig:30 j ] [30]))) 366 {*m68k.md:6326} (insn_list:REG_DEP_TRUE 16 (insn_list:REG_DEP_TRUE 13 (nil))\
)   
    (expr_list:REG_DEAD (reg:HI 0 %d0 [34])
        (expr_list:REG_DEAD (reg/v:SI 1 %d1 [orig:30 j ] [30])
            (nil))))

and emitted this part:
(insn 40 16 41 0 (set (reg:SI 2 %d2)
        (sign_extend:SI (reg:HI 0 %d0 [34]))) 65 {*68k_extendhisi2} (nil)
    (nil))

which isn't a valid m68k insn.

Ignoring for a moment, that the insn should have been covered by
m68k.md:1868 (so register allocation would have done to fit) had
that pattern had operands in canonical order, reload *must* handle
the new push_reload points introduced by r106804, by emitting the
unop insn in a way that "works".  (For anyone wanting to fix the
m68k.md:1868 pattern, you probably need to investigate and fix
whatever emits the non-canonical version.)

Before the patch below, the new push_reload points for SIGN_EXTEND,
ZERO_EXTEND and TRUNCATE introduced by r106804, dropped all the way
in gen_reload through to the "otherwise, just write (set OUT IN) and
hope for the best" case.  For that to work, all unop insns must be
two-operand for all register choices; it doesn't work for insn 40
above and "*68k_extendhisi2", with in=(sign_extend:SI (reg:HI 0 %d0
[34]))), out=(reg:SI 2 %d2) and constraints "=*d,a" and "0,rmS"
respectively.  As gen_reload is very much an ad-hoc construction
(see comment where I added a paragraph), I think what's best is to
just add what works for m68k; make source and destination registers
the same if the straightforward SET is invalid.  I also noticed a
bit of code duplication which I moved to a separate function instead
of adding more copies.

There's no m68k simulator in src/sim, but I successfully built a
m68k-elf toolchain with this patch (where previously it failed as in
the PR), except I had to disable fortran, where a similar-looking
(but with different cause) bug is exposed: see comment #5 in the PR.
I also ran the testsuite with --target=board=fc which is wrong, but
at least helped to test the compile-part of the testsuite.  I also
cross-tested from x86_64-unknown-linux-gnu to mmix-knuth-mmixware,
cris-axis-elf, cris-axis-linux-gnu and bootstrapped and tested
hosted x86_64-unknown-linux-gnu (FC4) and powerpc-unknown-linux-gnu
(FC4).

Ok to commit?

:ADDPATCH middle-end:

gcc/:
	PR middle-end/24912
	PR middle-end/24750
	* reload.c (find_reloads_address_1): Mention dependency on
	gen_reload.
	* reload1.c (gen_reload): For IN with an unary operation, try
	moving inner expression to OUT if trivial SET is not valid.
	Confirm that the result is valid.  Move common code block into...
	(emit_insn_if_valid_for_reload): New function.

gcc/testsuite/:
	PR middle-end/24912
	* gcc.dg/torture/pr24912-1.c: New test.
	
--- /dev/null	2004-02-23 22:02:56.000000000 +0100
+++ gcc.dg/torture/pr24912-1.c	2005-11-18 20:52:44.639835681 +0100
@@ -0,0 +1,10 @@
+void foo(void);
+void
+bar (unsigned char *p)
+{
+  int j;
+  j = *(p) ;
+  j += ((signed char) (*p) ) << 8;
+  if (j)
+    foo();
+}
Index: reload.c
===================================================================
--- reload.c	(revision 107118)
+++ reload.c	(working copy)
@@ -5311,7 +5311,9 @@
    occurs as part of an address.
    Also, this is not fully machine-customizable; it works for machines
    such as VAXen and 68000's and 32000's, but other possible machines
-   could have addressing modes that this does not handle right.  */
+   could have addressing modes that this does not handle right.
+   If you add push_reload calls here, you need to make sure gen_reload
+   handles those cases gracefully.  */
 
 static int
 find_reloads_address_1 (enum machine_mode mode, rtx x, int context,
Index: reload1.c
===================================================================
--- reload1.c	(revision 107118)
+++ reload1.c	(working copy)
@@ -437,6 +437,7 @@
 static void copy_eh_notes (rtx, rtx);
 static int reloads_conflict (int, int);
 static rtx gen_reload (rtx, rtx, int, enum reload_type);
+static rtx emit_insn_if_valid_for_reload (rtx);
 
 /* Initialize the reload pass once per compilation.  */
 
@@ -7449,6 +7450,32 @@
   IOR_HARD_REG_SET (reg_reloaded_dead, reg_reloaded_died);
 }
 
+/* Go through the motions to emit INSN and test if it is strictly valid.
+   Return the emitted insn if valid, else return NULL.  */
+
+static rtx
+emit_insn_if_valid_for_reload (rtx insn)
+{
+  rtx last = get_last_insn ();
+  int code;
+
+  insn = emit_insn (insn);
+  code = recog_memoized (insn);
+
+  if (code >= 0)
+    {
+      extract_insn (insn);
+      /* We want constrain operands to treat this insn strictly in its
+	 validity determination, i.e., the way it would after reload has
+	 completed.  */
+      if (constrain_operands (1))
+	return insn;
+    }
+
+  delete_insns_since (last);
+  return NULL;
+}
+
 /* Emit code to perform a reload from IN (which may be a reload register) to
    OUT (which may also be a reload register).  IN or OUT is from operand
    OPNUM with reload type TYPE.
@@ -7485,6 +7512,12 @@
      trying to emit a single insn to perform the add.  If it is not valid,
      we use a two insn sequence.
 
+     Or we can be asked to reload an unary operand that was a fragment of
+     an addressing mode, into a register.  If it isn't recognized as-is,
+     we try making the unop operand and the reload-register the same:
+     (set reg:X (unop:X expr:Y))
+     -> (set reg:Y expr:Y) (set reg:X (unop:X reg:Y)).
+
      Finally, we could be called to handle an 'o' constraint by putting
      an address into a register.  In that case, we first try to do this
      with a named pattern of "reload_load_address".  If no such pattern
@@ -7542,21 +7575,10 @@
       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);
+      insn = emit_insn_if_valid_for_reload (gen_rtx_SET (VOIDmode, out, in));
+      if (insn)
+	return insn;
 
-      if (code >= 0)
-	{
-	  extract_insn (insn);
-	  /* We want constrain operands to treat this insn strictly in
-	     its validity determination, i.e., the way it would after reload
-	     has completed.  */
-	  if (constrain_operands (1))
-	    return insn;
-	}
-
-      delete_insns_since (last);
-
       /* If that failed, we must use a conservative two-insn sequence.
 
 	 Use a move to copy one operand into the reload register.  Prefer
@@ -7591,30 +7613,18 @@
       if (rtx_equal_p (op0, op1))
 	op1 = out;
 
-      insn = emit_insn (gen_add2_insn (out, op1));
+      insn = emit_insn_if_valid_for_reload (gen_add2_insn (out, op1));
+      if (insn)
+	{
+	  /* Add a REG_EQUIV note so that find_equiv_reg can find it.  */
+	  REG_NOTES (insn)
+	    = gen_rtx_EXPR_LIST (REG_EQUIV, in, REG_NOTES (insn));
+	  return insn;
+	}
 
       /* If that failed, copy the address register to the reload register.
 	 Then add the constant to the reload register.  */
 
-      code = recog_memoized (insn);
-
-      if (code >= 0)
-	{
-	  extract_insn (insn);
-	  /* We want constrain operands to treat this insn strictly in
-	     its validity determination, i.e., the way it would after reload
-	     has completed.  */
-	  if (constrain_operands (1))
-	    {
-	      /* Add a REG_EQUIV note so that find_equiv_reg can find it.  */
-	      REG_NOTES (insn)
-		= gen_rtx_EXPR_LIST (REG_EQUIV, in, REG_NOTES (insn));
-	      return insn;
-	    }
-	}
-
-      delete_insns_since (last);
-
       gen_reload (out, op1, opnum, type);
       insn = emit_insn (gen_add2_insn (out, op0));
       REG_NOTES (insn) = gen_rtx_EXPR_LIST (REG_EQUIV, in, REG_NOTES (insn));
@@ -7643,7 +7653,44 @@
       gen_reload (out, loc, opnum, type);
     }
 #endif
+  else if (REG_P (out) && UNARY_P (in))
+    {
+      rtx insn;
+      rtx op1;
+      rtx out_moded;
+      rtx set;
 
+      /* First, try a plain SET.  */
+      set = emit_insn_if_valid_for_reload (gen_rtx_SET (VOIDmode, out, in));
+      if (set)
+	return set;
+
+      /* If that failed, move the inner operand to the reload
+	 register, and try the same unop with the inner expression
+	 replaced with the reload register.  */
+      op1 = XEXP (in, 0);
+
+      if (GET_MODE (op1) != GET_MODE (out))
+	out_moded = gen_rtx_REG (GET_MODE (op1), REGNO (out));
+      else
+	out_moded = out;
+
+      gen_reload (out_moded, op1, opnum, type);
+
+      insn
+	= gen_rtx_SET (VOIDmode, out,
+		       gen_rtx_fmt_e (GET_CODE (in), GET_MODE (in),
+				      out_moded));
+      insn = emit_insn_if_valid_for_reload (insn);
+      if (insn)
+	{
+	  REG_NOTES (insn)
+	    = gen_rtx_EXPR_LIST (REG_EQUIV, in, REG_NOTES (insn));
+	  return insn;
+	}
+
+      fatal_insn ("Failure trying to reload:", set);
+    }
   /* If IN is a simple operand, use gen_move_insn.  */
   else if (OBJECT_P (in) || GET_CODE (in) == SUBREG)
     emit_insn (gen_move_insn (out, in));

brgds, H-P