This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, MIPS] Merge clear_upper32 patterns into "and" patterns
- From: Adam Nemet <anemet at caviumnetworks dot com>
- To: Richard Sandiford <rdsandiford at googlemail dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Mon, 10 Aug 2009 11:15:54 -0700
- Subject: Re: [PATCH, MIPS] Merge clear_upper32 patterns into "and" patterns
- References: <19067.7284.223916.176193@ropi.home> <87hbwhhth5.fsf@firetop.home>
Richard,
Thanks very much for looking at patch.
Richard Sandiford writes:
> Adam Nemet <anemet@caviumnetworks.com> writes:
> - change the insns to use "nonmemory_operand" for operand 2
This does not work for mips16. 0xffff_ffff is not a legitimate constant so
the pattern would never match. Instead, I put the and_operand predicate back
and I used that.
> > --- 313,320 ----
> > ;; scheduling type to be "multi" instead.
> > (define_attr "move_type"
> > "unknown,load,fpload,store,fpstore,mtc,mfc,mthilo,mfhilo,move,fmove,
> > ! const,constN,signext,zextsi,zextr,logical,arith,sll0,andi,loadpool,
> > ! shift_shift,lui_movf"
> > (const_string "unknown"))
> >
> > ;; Main data type used by the insn
>
> You need to update the commentary. I'm not sure I really get the choice
> of names. ("r" for register? But "zextsi" is a register-to-register
> extension too.)
No the "r" was meant to distinguish between extend and extract, not very
intuitively, apparently :).
> How about "zeroext" for the MIPS16 one, since these insns are zero
> equivalents of the "signext" ones? Or perhaps we should just rename
> signext to "seb_seh", use "zeb_zeh" for the MIPS16E alternatives, and
> "ext_dext" for the MIPS32/64r2 alternatives.
Yes, I like the actual instruction names. I used that. I didn't change
seb_seh, though.
> Hmm, thinking more about this: do the ZEB and ZEH cases ever actually
> trigger? I would have expected an SImode AND to be represented as a
> zero extension in all cases. (I was still in the mindset of using them
> for DImode when I brought them up originally.)
>
> [...]
>
> Even if the ZEB and ZEH cases do trigger, I think it'd be OK to drop
> them if the use of AND rather than ZERO_EXTEND looks like a bug.
They do trigger in a few places but rather than opening up another can of
worms I decided to remove the zeb/zeh variants for now. The two places I've
looked at so far can probably be fixed in the middle-end and we can always put
these variants back if it turns out that reload needs the extra flexibility.
> If not -- and if that means we can simplify the MIPS16 case -- then
> I'd prefer to move the ISA_HAS_EXT_INS check into low_bitmask_operand
> and avoid the use of the enabled attribute. (And TBH, I'd be a lot
> happier with that version too.) and_reg_operand would simplify to
> something like:
I hope I didn't misunderstand this part; you essential prefer the "Yx"
constraint to have an empty domain if !ISA_HAS_EXT_INS rather than disabling
it through the "enabled" attribute.
With that here is the new version.
Bootstrapped and regtested on mips64octeon-linux. Regtested on
mipsisa64r2-elf and mipsisa64-elfoabi{,-mips16,-mips16/-mips4}.
OK to install?
Adam
* config/mips/predicates.md (qi_mask_operand, hi_mask_operand,
si_mask_operand, and_load_operand, low_bitmask_operand,
and_reg_operand, and_operand): New predicates.
* config/mips/constraints.md (Yb, Yh, Yw, Yz): New constraints.
* config/mips/mips.c (and_operands_ok): New function.
* config/mips/mips-protos.h (and_operands_ok): Declare it.
* config/mips/mips.md (move_type): Add ext_ins and logical.
(type): Handle them.
(and<mode>3): Use and_reg_operand as the second operand's
predicate.
(*and<mode>3): Add alternatives for lbu, lhu, lwu, <d>ext and
shift_shift. Remove commutative constraint modifier.
(*and<mode>3_mips16): Add alternatives for lbu, lhu, lwu and
shift_shift.
(*clear_upper32_dext): Remove define_insn_and_split.
(*clear_upper32): Turn this define_insn_and_split ...
(splitter for ANDing register with 0xffff_ffff): .. into this.
testsuite/
* gcc.target/mips/ext-5.c: New test.
* gcc.target/mips/ext-6.c: New test.
* gcc.target/mips/ext-7.c: New test.
* gcc.target/mips/ext-8.c: New test.
* gcc.target/mips/extend-2.c: New test.
Index: gcc/config/mips/predicates.md
===================================================================
*** gcc.orig/config/mips/predicates.md 2009-07-31 17:35:02.000000000 -0700
--- gcc/config/mips/predicates.md 2009-08-09 17:08:47.000000000 -0700
*************** (define_predicate "const_0_or_1_operand"
*** 76,81 ****
--- 76,114 ----
(ior (match_test "op == CONST0_RTX (GET_MODE (op))")
(match_test "op == CONST1_RTX (GET_MODE (op))"))))
+ (define_predicate "qi_mask_operand"
+ (and (match_code "const_int")
+ (match_test "UINTVAL (op) == 0xff")))
+
+ (define_predicate "hi_mask_operand"
+ (and (match_code "const_int")
+ (match_test "UINTVAL (op) == 0xffff")))
+
+ (define_predicate "si_mask_operand"
+ (and (match_code "const_int")
+ (match_test "UINTVAL (op) == 0xffffffff")))
+
+ (define_predicate "and_load_operand"
+ (ior (match_operand 0 "qi_mask_operand")
+ (match_operand 0 "hi_mask_operand")
+ (match_operand 0 "si_mask_operand")))
+
+ (define_predicate "low_bitmask_operand"
+ (and (match_test "ISA_HAS_EXT_INS")
+ (match_code "const_int")
+ (match_test "low_bitmask_len (mode, INTVAL (op)) > 16")))
+
+ (define_predicate "and_reg_operand"
+ (ior (match_operand 0 "register_operand")
+ (and (match_test "!TARGET_MIPS16")
+ (match_operand 0 "const_uns_arith_operand"))
+ (match_operand 0 "low_bitmask_operand")
+ (match_operand 0 "si_mask_operand")))
+
+ (define_predicate "and_operand"
+ (ior (match_operand 0 "and_load_operand")
+ (match_operand 0 "and_reg_operand")))
+
(define_predicate "d_operand"
(and (match_code "reg")
(match_test "TARGET_MIPS16
Index: gcc/config/mips/constraints.md
===================================================================
*** gcc.orig/config/mips/constraints.md 2009-07-31 17:35:02.000000000 -0700
--- gcc/config/mips/constraints.md 2009-08-09 14:20:50.000000000 -0700
*************** (define_constraint "YB"
*** 215,217 ****
--- 215,233 ----
A signed 10-bit constant."
(and (match_code "const_int")
(match_test "IMM10_OPERAND (ival)")))
+
+ (define_constraint "Yb"
+ "@internal"
+ (match_operand 0 "qi_mask_operand"))
+
+ (define_constraint "Yh"
+ "@internal"
+ (match_operand 0 "hi_mask_operand"))
+
+ (define_constraint "Yw"
+ "@internal"
+ (match_operand 0 "si_mask_operand"))
+
+ (define_constraint "Yx"
+ "@internal"
+ (match_operand 0 "low_bitmask_operand"))
Index: gcc/config/mips/mips-protos.h
===================================================================
*** gcc.orig/config/mips/mips-protos.h 2009-07-31 17:35:02.000000000 -0700
--- gcc/config/mips/mips-protos.h 2009-08-09 14:06:40.000000000 -0700
*************** extern bool mips16e_save_restore_pattern
*** 316,321 ****
--- 316,322 ----
extern bool mask_low_and_shift_p (enum machine_mode, rtx, rtx, int);
extern int mask_low_and_shift_len (enum machine_mode, rtx, rtx);
+ extern bool and_operands_ok (enum machine_mode, rtx, rtx);
union mips_gen_fn_ptrs
{
Index: gcc/config/mips/mips.c
===================================================================
*** gcc.orig/config/mips/mips.c 2009-07-31 17:35:02.000000000 -0700
--- gcc/config/mips/mips.c 2009-08-09 17:38:38.000000000 -0700
*************** mask_low_and_shift_p (enum machine_mode
*** 6781,6786 ****
--- 6781,6798 ----
return IN_RANGE (mask_low_and_shift_len (mode, mask, shift), 1, maxlen);
}
+ /* Return true iff OP1 and OP2 are valid operands together for the
+ *and<MODE>3 and *and<MODE>3_mips16 patterns. For the cases to consider,
+ see the table in the comment before the pattern. */
+
+ bool
+ and_operands_ok (enum machine_mode mode, rtx op1, rtx op2)
+ {
+ return (memory_operand (op1, mode)
+ ? and_load_operand (op2, mode)
+ : and_reg_operand (op2, mode));
+ }
+
/* The canonical form of a mask-low-and-shift-left operation is
(and (ashift X SHIFT) MASK) where MASK has the lower SHIFT number of bits
cleared. Thus we need to shift MASK to the right before checking if it
Index: gcc/config/mips/mips.md
===================================================================
*** gcc.orig/config/mips/mips.md 2009-07-31 17:38:00.000000000 -0700
--- gcc/config/mips/mips.md 2009-08-10 11:12:09.000000000 -0700
*************** (define_attr "jal_macro" "no,yes"
*** 301,306 ****
--- 301,307 ----
;; sll0 "sll DEST,SRC,0", which on 64-bit targets is guaranteed
;; to produce a sign-extended DEST, even if SRC is not
;; properly sign-extended
+ ;; ext_ins EXT, DEXT, INS or DINS instruction
;; andi a single ANDI instruction
;; loadpool move a constant into a MIPS16 register by loading it
;; from the pool
*************** (define_attr "jal_macro" "no,yes"
*** 313,319 ****
;; scheduling type to be "multi" instead.
(define_attr "move_type"
"unknown,load,fpload,store,fpstore,mtc,mfc,mthilo,mfhilo,move,fmove,
! const,constN,signext,arith,sll0,andi,loadpool,shift_shift,lui_movf"
(const_string "unknown"))
;; Main data type used by the insn
--- 314,321 ----
;; scheduling type to be "multi" instead.
(define_attr "move_type"
"unknown,load,fpload,store,fpstore,mtc,mfc,mthilo,mfhilo,move,fmove,
! const,constN,signext,ext_ins,logical,arith,sll0,andi,loadpool,
! shift_shift,lui_movf"
(const_string "unknown"))
;; Main data type used by the insn
*************** (define_attr "type"
*** 408,414 ****
--- 410,418 ----
(eq_attr "move_type" "fmove") (const_string "fmove")
(eq_attr "move_type" "loadpool") (const_string "load")
(eq_attr "move_type" "signext") (const_string "signext")
+ (eq_attr "move_type" "ext_ins") (const_string "arith")
(eq_attr "move_type" "arith") (const_string "arith")
+ (eq_attr "move_type" "logical") (const_string "logical")
(eq_attr "move_type" "sll0") (const_string "shift")
(eq_attr "move_type" "andi") (const_string "logical")
*************** (define_insn "one_cmpl<mode>2"
*** 2561,2591 ****
(define_expand "and<mode>3"
[(set (match_operand:GPR 0 "register_operand")
(and:GPR (match_operand:GPR 1 "register_operand")
! (match_operand:GPR 2 "uns_arith_operand")))]
""
{
if (TARGET_MIPS16)
operands[2] = force_reg (<MODE>mode, operands[2]);
})
(define_insn "*and<mode>3"
! [(set (match_operand:GPR 0 "register_operand" "=d,d")
! (and:GPR (match_operand:GPR 1 "register_operand" "%d,d")
! (match_operand:GPR 2 "uns_arith_operand" "d,K")))]
! "!TARGET_MIPS16"
! "@
! and\t%0,%1,%2
! andi\t%0,%1,%x2"
! [(set_attr "type" "logical")
(set_attr "mode" "<MODE>")])
(define_insn "*and<mode>3_mips16"
! [(set (match_operand:GPR 0 "register_operand" "=d")
! (and:GPR (match_operand:GPR 1 "register_operand" "%0")
! (match_operand:GPR 2 "register_operand" "d")))]
! "TARGET_MIPS16"
! "and\t%0,%2"
! [(set_attr "type" "logical")
(set_attr "mode" "<MODE>")])
(define_expand "ior<mode>3"
--- 2565,2664 ----
(define_expand "and<mode>3"
[(set (match_operand:GPR 0 "register_operand")
(and:GPR (match_operand:GPR 1 "register_operand")
! (match_operand:GPR 2 "and_reg_operand")))]
""
{
if (TARGET_MIPS16)
operands[2] = force_reg (<MODE>mode, operands[2]);
})
+ ;; The middle-end is not allowed to convert ANDing with 0xffff_ffff into a
+ ;; zero_extendsidi2 because of TRULY_NOOP_TRUNCATION, so handle these here.
+ ;; Note that this variant does not trigger for SI mode because we require
+ ;; a 64-bit HOST_WIDE_INT and 0xffff_ffff wouldn't be a canonical
+ ;; sign-extended SImode value.
+ ;;
+ ;; These are possible combinations for operand 1 and 2. The table
+ ;; includes both MIPS and MIPS16 cases. (r=register, mem=memory,
+ ;; 16=MIPS16, x=match, S=split):
+ ;;
+ ;; \ op1 r/EXT r/!EXT mem r/16 mem/16
+ ;; op2
+ ;;
+ ;; andi x x
+ ;; 0xff x x x x
+ ;; 0xffff x x x x
+ ;; 0xffff_ffff x S x S x
+ ;; low-bitmask x
+ ;; register x x
+ ;; register =op1 x
+ ;;
+ ;; The 0xffff_ffff case does not trigger with SI mode because we require
+ ;; a 64-bit HOST_WIDE_INT and 0xffff_ffff wouldn't be a canonical
+ ;; sign-extended SImode value.
+
(define_insn "*and<mode>3"
! [(set (match_operand:GPR 0 "register_operand" "=d,d,d,d,d,d,d")
! (and:GPR (match_operand:GPR 1 "nonimmediate_operand" "o,o,W,d,d,d,d")
! (match_operand:GPR 2 "and_operand" "Yb,Yh,Yw,K,Yx,Yw,d")))]
! "!TARGET_MIPS16 && and_operands_ok (<MODE>mode, operands[1], operands[2])"
! {
! int len;
!
! switch (which_alternative)
! {
! case 0:
! operands[1] = gen_lowpart (QImode, operands[1]);
! return "lbu\t%0,%1";
! case 1:
! operands[1] = gen_lowpart (HImode, operands[1]);
! return "lhu\t%0,%1";
! case 2:
! operands[1] = gen_lowpart (SImode, operands[1]);
! return "lwu\t%0,%1";
! case 3:
! return "andi\t%0,%1,%x2";
! case 4:
! len = low_bitmask_len (<MODE>mode, INTVAL (operands[2]));
! operands[2] = GEN_INT (len);
! return "<d>ext\t%0,%1,0,%2";
! case 5:
! return "#";
! case 6:
! return "and\t%0,%1,%2";
! default:
! gcc_unreachable ();
! }
! }
! [(set_attr "move_type" "load,load,load,andi,ext_ins,shift_shift,logical")
(set_attr "mode" "<MODE>")])
(define_insn "*and<mode>3_mips16"
! [(set (match_operand:GPR 0 "register_operand" "=d,d,d,d,d")
! (and:GPR (match_operand:GPR 1 "nonimmediate_operand" "%o,o,W,d,0")
! (match_operand:GPR 2 "and_operand" "Yb,Yh,Yw,Yw,d")))]
! "TARGET_MIPS16 && and_operands_ok (<MODE>mode, operands[1], operands[2])"
! {
! switch (which_alternative)
! {
! case 0:
! operands[1] = gen_lowpart (QImode, operands[1]);
! return "lbu\t%0,%1";
! case 1:
! operands[1] = gen_lowpart (HImode, operands[1]);
! return "lhu\t%0,%1";
! case 2:
! operands[1] = gen_lowpart (SImode, operands[1]);
! return "lwu\t%0,%1";
! case 3:
! return "#";
! case 4:
! return "and\t%0,%2";
! default:
! gcc_unreachable ();
! }
! }
! [(set_attr "move_type" "load,load,load,shift_shift,logical")
(set_attr "mode" "<MODE>")])
(define_expand "ior<mode>3"
*************** (define_insn "*zero_extendsidi2_dext"
*** 2778,2821 ****
[(set_attr "move_type" "arith,load")
(set_attr "mode" "DI")])
! ;; Combine is not allowed to convert this insn into a zero_extendsidi2
! ;; because of TRULY_NOOP_TRUNCATION.
! (define_insn_and_split "*clear_upper32"
! [(set (match_operand:DI 0 "register_operand" "=d,d")
! (and:DI (match_operand:DI 1 "nonimmediate_operand" "d,W")
(const_int 4294967295)))]
! "TARGET_64BIT && !ISA_HAS_EXT_INS"
! {
! if (which_alternative == 0)
! return "#";
!
! operands[1] = gen_lowpart (SImode, operands[1]);
! return "lwu\t%0,%1";
! }
! "&& reload_completed && REG_P (operands[1])"
[(set (match_dup 0)
(ashift:DI (match_dup 1) (const_int 32)))
(set (match_dup 0)
! (lshiftrt:DI (match_dup 0) (const_int 32)))]
! ""
! [(set_attr "move_type" "shift_shift,load")
! (set_attr "mode" "DI")])
!
! (define_insn "*clear_upper32_dext"
! [(set (match_operand:DI 0 "register_operand" "=d,d")
! (and:DI (match_operand:DI 1 "nonimmediate_operand" "d,W")
! (const_int 4294967295)))]
! "TARGET_64BIT && ISA_HAS_EXT_INS"
! {
! if (which_alternative == 0)
! return "dext\t%0,%1,0,32";
!
! operands[1] = gen_lowpart (SImode, operands[1]);
! return "lwu\t%0,%1";
! }
! [(set_attr "move_type" "arith,load")
! (set_attr "mode" "DI")])
(define_expand "zero_extend<SHORT:mode><GPR:mode>2"
[(set (match_operand:GPR 0 "register_operand")
--- 2851,2868 ----
[(set_attr "move_type" "arith,load")
(set_attr "mode" "DI")])
! ;; See the comment before the *and<mode>3 pattern why this is generated by
! ;; combine.
! (define_split
! [(set (match_operand:DI 0 "register_operand")
! (and:DI (match_operand:DI 1 "register_operand")
(const_int 4294967295)))]
! "TARGET_64BIT && !ISA_HAS_EXT_INS && reload_completed"
[(set (match_dup 0)
(ashift:DI (match_dup 1) (const_int 32)))
(set (match_dup 0)
! (lshiftrt:DI (match_dup 0) (const_int 32)))])
(define_expand "zero_extend<SHORT:mode><GPR:mode>2"
[(set (match_operand:GPR 0 "register_operand")
Index: gcc/testsuite/gcc.target/mips/ext-5.c
===================================================================
*** /dev/null 1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/gcc.target/mips/ext-5.c 2009-08-01 20:46:31.000000000 -0700
***************
*** 0 ****
--- 1,11 ----
+ /* For MIPS32r2 use EXT when ANDing with low-order bitmasks. */
+ /* { dg-do compile } */
+ /* { dg-options "-O isa_rev>=2" } */
+ /* { dg-final { scan-assembler "\text\t" } } */
+ /* { dg-final { scan-assembler-not "\tandi?\t" } } */
+
+ NOMIPS16 unsigned
+ f (unsigned i)
+ {
+ return i & 0x7ffffff;
+ }
Index: gcc/testsuite/gcc.target/mips/ext-6.c
===================================================================
*** /dev/null 1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/gcc.target/mips/ext-6.c 2009-08-01 20:46:50.000000000 -0700
***************
*** 0 ****
--- 1,11 ----
+ /* For MIPS64r2 use DEXT when ANDing with low-order bitmasks. */
+ /* { dg-do compile } */
+ /* { dg-options "-O isa_rev>=2 -mgp64" } */
+ /* { dg-final { scan-assembler "\tdext\t" } } */
+ /* { dg-final { scan-assembler-not "\tandi?\t" } } */
+
+ NOMIPS16 unsigned long long
+ f (unsigned long long i)
+ {
+ return i & 0x7ffffffffff;
+ }
Index: gcc/testsuite/gcc.target/mips/ext-7.c
===================================================================
*** /dev/null 1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/gcc.target/mips/ext-7.c 2009-08-01 20:47:34.000000000 -0700
***************
*** 0 ****
--- 1,11 ----
+ /* No need to use ext if we can use andi. */
+ /* { dg-do compile } */
+ /* { dg-options "-O isa_rev>=2" } */
+ /* { dg-final { scan-assembler "\tandi\t" } } */
+ /* { dg-final { scan-assembler-not "\td?ext\t" } } */
+
+ NOMIPS16 unsigned
+ f (unsigned i)
+ {
+ return i & 0x7fff;
+ }
Index: gcc/testsuite/gcc.target/mips/ext-8.c
===================================================================
*** /dev/null 1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/gcc.target/mips/ext-8.c 2009-08-06 11:04:35.000000000 -0700
***************
*** 0 ****
--- 1,11 ----
+ /* Also make sure we don't use ext for MIPS*r1. */
+ /* { dg-do compile } */
+ /* { dg-options "-O isa_rev<=1" } */
+ /* { dg-final { scan-assembler "\tand\t" } } */
+ /* { dg-final { scan-assembler-not "\td?ext\t" } } */
+
+ unsigned
+ f (unsigned i)
+ {
+ return i & 0x7fffff;
+ }
Index: gcc/testsuite/gcc.target/mips/extend-2.c
===================================================================
*** /dev/null 1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/gcc.target/mips/extend-2.c 2009-08-06 11:02:22.000000000 -0700
***************
*** 0 ****
--- 1,12 ----
+ /* Check the shift_shift alternative of the AND patterns. */
+ /* { dg-do compile } */
+ /* { dg-options "-O isa_rev<=1 -mgp64" } */
+ /* { dg-final { scan-assembler "\tdsrl\t" } } */
+ /* { dg-final { scan-assembler "\tdsll\t" } } */
+ /* { dg-final { scan-assembler-not "\td?ext\t" } } */
+
+ unsigned long long
+ f (unsigned long long i)
+ {
+ return i & 0xffffffff;
+ }