Re: HI to SI mode miss conversion on MIPS.

[Jeffrey A Law <law at cygnus dot com>]

  In message <20000210204436.A24896@cpopetz.com>you write:
  > On Thu, Feb 10, 2000 at 09:03:25PM +0900, Hiroyuki Machida wrote:
  > > 
  > > Hi,
  > > I found the problem about HI to SI mode conversion on MIPS.
  > > 
  > > REPEATED BY
  > > 	gcc -O2 -g -df -dc hisi.c -o hisi
  > > 	./hisi
  > > 
  > > VERSION
  > > 	gcc version 2.95.2 19991024 (release) for MIPS
  > > 	gcc version 2.96 20000124 (experimental) for MIPS
  > > 
  > > The attached test program shows that gcc produces the incorrect 
  > > object for swapping lower and upper byte. This test code derived 
  > > from linux kernel linux/drivers/net/slhc.c.
  > > 
  > > The test swaps 0x0000235f twice, expceted results is 0x0000235f. But
  > > you will get 0x005f235f on MIPSEB box. (This problem is occured even
  > > if on MIPSEL box)
  > > 
  > > I tracked down in rough. In my observation, required "zero_extend"
  > > or somthing was vanished on the "combination-phase".
  > > 
  > > Followings are interested portions of the test programe and
  > > corresponding RTL codes. The problem is occured at the line 65 of
  > > the C source code.
  > > 
  > > In the hisi.c.08.flow, RTLs look good. But #78, #86 and #99 in the
  > > hisi.c.09.combine produce incorrect result.  
  > > In my concern, The RTL corresspoindig to #95 in the hisi.c.08.flow,
  > > 	(set (reg:SI 111) (zero_extend:SI (reg:HI 111)))
  > > must be put at the next of
  > > #86 (set (subreg:SI (reg:HI 111) 0) (ior:SI (reg:SI 114) (reg:SI 117))) 
  > > in the hisi.c.09.combine.
  > > 
  > > The attached patch is a workaround for this problem. Does anyone
  > > have better solution or real fix ?  
  > > 
  > 
  > Yes, I think so.  The problem seems to be in force_to_mode for ANDs with
  > a constant.  There is the following block to make cheaper constants:
  > 
  >   if (GET_CODE (x) == AND && GET_CODE (XEXP (x, 1)) == CONST_INT
  >       && GET_MODE_MASK (GET_MODE (x)) != mask
  >       && GET_MODE_BITSIZE (GET_MODE (x)) <= HOST_BITS_PER_WIDE_INT)
  >     {
  >       HOST_WIDE_INT cval = (INTVAL (XEXP (x, 1))
  > 			    | (GET_MODE_MASK (GET_MODE (x)) & ~ mask));
  >       int width = GET_MODE_BITSIZE (GET_MODE (x));
  >       rtx y;
  >       
  >       /* If MODE is narrower that HOST_WIDE_INT and CVAL is a negative
  > 	 number, sign extend it.  */ 
  >       if (width > 0 && width < HOST_BITS_PER_WIDE_INT
  > 	  && (cval & ((HOST_WIDE_INT) 1 << (width - 1))) != 0)
	cval |= (HOST_WIDE_INT) -1 << width;
  >   
  >       y = gen_binary (AND, GET_MODE (x), XEXP (x, 0), GEN_INT (cval));
  >       if (GET_CODE (y) == AND && rtx_cost (y, SET) < rtx_cost (x, SET))
  > 	x = y;    
  >     }
  > 
  > 
  > So let's say we have something like this:
  > 
  > (and:SI (ashift:SI (reg:SI 111)
  >         (const_int 8 [0x8]))
  >     (const_int 65280 [0xff00]))
  > 
  > and we are forcing to HI, so mask is 0xffff.  This makes cval 0xffffff00.
  > 
  > But even if combine knows nothing about reg 111, so that nonzero_bits for
  > it is 0xffffffff, then nonzero_bits for the ashift will be 0xffffff00, and
  > gen_binary will ellide the AND, returning the ashift, which is wrong.
  > 
  > We need to check that the result of gen_binary is still an AND before
  > using it.
Maybe I'm just dense today, but I don't see how these two expressions are
not equivalent:

(and:SI (ashift:SI (reg:SI 108)
         (const_int 8 [0x8]))
    (const_int 65280 [0xff00]))

(subreg:HI (ashift:SI (reg:SI 108)
        (const_int 8 [0x8])) 0)


The first is X in the code fragment you posted.  The second is the return
value from force_to_mode.  Note how we've wrapped the expression in a subreg
which indicates that we don't care about bits outside of HImode.

jeff