This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Allow widening the mode of right-shifts in simplify_shift_const_1


Adam Nemet wrote:
This is a follow-up to
<http://gcc.gnu.org/ml/gcc-patches/2009-05/msg01253.html>.  As I mentioned
there, the truncate after the sign-extraction is redundant.  However I missed
at the time that so is the subsequent sign-extension.  This patch removes the
sign-extension.

Compound operations are optimized in their expanded from in combine and
redundant sign-extensions are normally removed in simplify_shift_const_1:

	case LSHIFTRT:
	case ASHIFT:
	case ROTATE:
	[...]
	      /* If this was (ashiftrt (ashift foo C1) C2) and FOO has more
		 than C1 high-order bits equal to the sign bit, we can convert
		 this to either an ASHIFT or an ASHIFTRT depending on the
		 two counts.

We cannot do this if VAROP's mode is not SHIFT_MODE. */

	      if (code == ASHIFTRT && first_code == ASHIFT
		  && GET_MODE (varop) == shift_mode
		  && (num_sign_bit_copies (XEXP (varop, 0), shift_mode)
		      > first_count))
		{
		  varop = XEXP (varop, 0);
		  count -= first_count;
		  if (count < 0)
		    {
		      count = -count;
		      code = ASHIFT;
		    }

		  continue;
		}

Using the new testcase, which extracts 14 bits, we start with these patterns:

7: (set (reg:DI 202)
        (sign_extract:DI (reg:DI 200)
            (const_int 14 [0xe])
            (const_int 32 [0x20])))

8: (set (reg:HI 203)
        (truncate:HI (reg:DI 202)))

9: (set (reg:SI 197)
        (sign_extend:SI (reg:HI 203)))

then fail to combine this:

Trying 8 -> 9:
Failed to match this instruction:
(set (reg:SI 197)
    (ashiftrt:SI (subreg:SI (ashift:DI (reg:DI 202)
                (const_int 16 [0x10])) 4)
        (const_int 16 [0x10])))


The subreg is in the way of merging the two shifts into nothing. (The truncate was turned into a subreg by the TARGET_MODE_REP_EXTENDED machinery.)

In general we're not allowed to widen the mode of right shifts because instead
of zero or the value of the sign-bit we'd bring in high-order bits.  This is
however allowed if we can prove that the high-order bits contain what would
normally be shifted in.

This is what my patch does first for ASHIFTRT.  Further patches will add the
other cases.  (I split this up to help bisecting.)  We can widen the mode of
an ASHIFTRT if the bits brought in are identical to the sign-bit of the
original mode.

My code makes it more explicit that we're always widening the mode here.  This
is guaranteed because MODE is initialized to RESULT_MODE and only set under
case SUBREG: if the new mode is wider than the original value of MODE.  So
MODE can never be narrower than RESULT_MODE.

The patch also reverts the work-around I had to install for the MIPS tests due
to this limitation.

Bootstrapped and regtested on {mips64octeon,x86_64}-linux and regtested on
mipsisa64r2-elf.

OK for trunk?

Adam


* combine.c (simplify_shift_const_1): Split code to determine shift_mode into ... (try_widen_shift_mode): ... here. Allow widening for ASHIFTRT if the new bits shifted in are identical to the old sign bit.

testsuite/
* gcc.target/mips/octeon-exts-7.c: New test.
* gcc.target/mips/octeon-exts-2.c: Revert previous change.
* gcc.target/mips/octeon-exts-5.c: Likewise.
OK.

jeff


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]