Re: My egcs patch for Alpha

Re: My egcs patch for Alpha

[Richard Henderson]

On Tue, Mar 17, 1998 at 11:34:31AM -0500, Richard Gorton wrote:
>       * fold-const.c (operand_equal_for_comparison_p): locally relax
>       width comparison to permit better (faster) unsigned vs. signed
>       comparisons.
>       * expmed.c (extract_bit_field): return subreg when target machine
>       has extzv patterns involving widened modes.

These two were taken care of by Kenner's follow-on patches, correct?
They seem to be, but just making sure.

> 	* alpha.md: modify extzv pattern and add a helper pattern to
> 	perform better byte extract/insert operations

I've modified the extzv pattern to not reject register operands,
though surely you didn't intend to kill my unaligned load bits.  ;-)

As for the helper pattern,

  (define_insn ""
    [(set (match_operand:DI 0 "register_operand" "=r")
        (ashift:DI (subreg:DI (zero_extend:SI (subreg:QI
 			(match_operand:SI 1 "register_operand" "r") 0)) 0)
                   (match_operand:DI 2 "mul8_operand" "I")))]
    "INTVAL (operands[2]) != 0"
    "insbl %1,%s2,%0"
    [(set_attr "type" "shift")])

this is quite ugly, and would also call for many other similar patterns
for inswl etc, and possibly other cases.

Rather than add something like this to the md file, better is to have
combine recognize this and have it simplified.  In this case, all that
is required to match the existing insbl pattern

  [(set (match_operand:DI 0 "register_operand" "=r")
        (ashift:DI (zero_extend:DI (match_operand:QI 1 "register_operand" "r"))
                   (match_operand:DI 2 "mul8_operand" "I")))]

is to realize that (subreg:DI (zero_extend:SI X) 0) can be simpified
to (zero_extend:DI X).

The following patch accomplishes this.

As an aside, can someone tell me why the test immediately above mine
tests to see if the new code is the same as the original pattern's
code?  It seems to me that if the substitution is valid now at all
it shouldn't matter what the previous code was, so there's no point
in testing that.

Along the same lines, is there any point in my test for paradoxical
subreg?  It now occurs to me that the high zeros of

  (subreg:SI (zero_extend:DI X))

is rather pointless.  We might even be able to get rid of the extension
as well.


r~


Wed Mar 18 16:26:06 1998  Richard Henderson  <rth@cygnus.com>

	* combine.c (make_compound_operation): Widen extensions through
	paradoxical subregs.

Index: combine.c
===================================================================
RCS file: /egcs/carton/cvsfiles/egcs/gcc/combine.c,v
retrieving revision 1.25
diff -c -p -d -r1.25 combine.c
*** combine.c	1998/03/18 07:17:51	1.25
--- combine.c	1998/03/19 00:23:48
*************** make_compound_operation (x, in_code)
*** 6038,6043 ****
--- 6038,6054 ----
  
  	  return newer;
  	}
+ 
+       /* If this is a paradoxical subreg, and the new code is a sign or
+ 	 zero extension, omit the subreg and widen the extension.  */
+       if (GET_MODE_SIZE (mode) > GET_MODE_SIZE (GET_MODE (tem))
+ 	  && (GET_CODE (tem) == SIGN_EXTEND
+ 	      || GET_CODE (tem) == ZERO_EXTEND)
+ 	  && subreg_lowpart_p (x))
+ 	{
+ 	  PUT_MODE (tem, mode);
+ 	  return tem;
+ 	}
        break;
        
      default:

Re: My egcs patch for Alpha

[Jim Wilson]

	As an aside, can someone tell me why the test immediately above mine
	tests to see if the new code is the same as the original pattern's
	code?

I think the assumption here is that if the code is the same, then the recursive
make_compound_operation call didn't do anything, and hence there is nothing
new to substitute in.

	Along the same lines, is there any point in my test for paradoxical
	subreg?

I don't see any need for this test either.

+ 	  PUT_MODE (tem, mode);

The patch looks OK to me, however, this PUT_MODE call looks unsafe.  We must
be able to undo all changes if the result fails to match the md file, but we
can't undo a PUT_MODE.  Hence this will fail if tem happens to be shared with
the original RTX for the insn.  You should use gen_rtx_combine instead.

Jim

Re: My egcs patch for Alpha

[Richard Henderson]

> I think the assumption here is that if the code is the same, then the
> recursive make_compound_operation call didn't do anything, and hence
> there is nothing new to substitute in.

Huh.  Though that really assumes that the code was simplified before, no?

> The patch looks OK to me, however, this PUT_MODE call looks unsafe.

Ok.  I've comitted a version that uses gen_rtx_combine and 
gen_lowpart_for_combine as needed.


r~