Re: PATCH COMMITTED: More lower-subreg.c patches

[Ian Lance Taylor <iant at google dot com>]

DJ Delorie <dj@redhat.com> writes:

> > If you tell me how to configure gcc and give me a test case, I will
> > look into the problem.
> 
> --target=m32c-elf
> 
> fstream-inst.cc in libstdc++-v3, m32cm variant.
> 
> I.e. I'm just trying to build gcc itself.
> 
> The m32cm or m32c cpus in the m32c-elf build are good testcases for
> this type of thing, because they have four 8-bit registers, two 16-bit
> registers, and five 24-bit address registers.  The registers can be
> combined in very non-orthogonal ways to make 16, 24, 32, 48, and 64
> bit registers.  Two of the address registers can be referenced as 16
> or 8 bit registers, but only the LSB.
> 
> The r8c and m16c cpus have 16 bit address registers (but a 20 bit
> $pc), but two of them can be concatenated to a 32 bit address
> register.

Thanks for sending the test case.

I believe this may be a bug in movhi_op insn in the m32c port.  It is
a general rule that if the operand predicates accept an operand, and
the insn predicate does as well, that there must be a constraint which
accepts the operand.  In this case the operand
    (subreg:HI (reg/f:PSI 8 sp) 0)
is accepted by the predicates, but, eventually,
    (reg:HI 8 sp)
is generated by reload and rejected by the constraints.  In this case
m32c_any_operand truly accepts any operand; it should reject
constructs which will not work.

The lower-subreg code is doing the right thing.  It generates this:

(insn 139 80 140 10 (set (reg/f:PSI 67)
        (reg/f:PSI 8 sp)) 172 {movpsi_op} (nil)
    (nil))

(insn 140 139 141 10 (set (reg:HI 65)
        (subreg:HI (reg/f:PSI 67) 0)) 171 {movhi_op} (nil)
    (nil))

(insn 141 140 142 10 (set (reg:HI 66 [+2 ])
        (subreg:HI (reg/f:PSI 67) 2)) 171 {movhi_op} (nil)
    (nil))

However, because the predicate for movhi_op accepts a subreg of the sp
register, CSE turns that into this:

(insn 139 80 140 10 (set (reg/f:PSI 67)
        (reg/f:PSI 8 sp)) 172 {movpsi_op} (nil)
    (nil))

(insn 140 139 141 10 (set (reg:HI 65)
        (subreg:HI (reg/f:PSI 8 sp) 0)) 171 {movhi_op} (nil)
    (nil))

(insn 141 140 142 10 (set (reg:HI 66 [+2 ])
        (subreg:HI (reg/f:PSI 8 sp) 2)) 171 {movhi_op} (nil)
    (nil))

And that leads to trouble down the road.


Despite that, I don't think it is necessary to fix m32c/mov.md to
avoid this problem.  The PARTIAL_INT modes are weird, and
lower-subreg.c should avoid tampering with them.

I'm currently testing this patch, which appears to fix the problem.  I
will commit it if it passes testing on i686-pc-linux-gnu.

Ian


2007-02-07  Ian Lance Taylor  <iant@google.com>

	* lower-subreg.c (simple_move): Reject PARTIAL_INT modes.


Index: lower-subreg.c
===================================================================
--- lower-subreg.c	(revision 121652)
+++ lower-subreg.c	(working copy)
@@ -136,6 +136,11 @@ simple_move (rtx insn)
 	  == BLKmode))
     return NULL_RTX;
 
+  /* Reject PARTIAL_INT modes.  They are used for processor specific
+     purposes and it's probably best not to tamper with them.  */
+  if (GET_MODE_CLASS (mode) == MODE_PARTIAL_INT)
+    return NULL_RTX;
+
   return set;
 }