Re: [ARM Patch 2/n]PR53447: optimizations of 64bit ALU operation with constant

[Ramana Radhakrishnan <ramana dot radhakrishnan at linaro dot org>]

On 28 May 2012 11:08, Carrot Wei <carrot@google.com> wrote:
> Hi
>
> This is the second part of the patches that deals with 64bit and. It directly
> extends the patterns anddi3, anddi3_insn and anddi3_neon to handle 64bit
> constant operands.
>

Comments about const_di_ok_for_op still apply from earlier review.

However I don't see and /ior / xor with constants that have either the
low or high parts set can't be expanded directly into ands of subregs
with moves of zero's or the original value ?

This and some of the other boolean operations should be targeting PR
target/53189 by the way.


Ramana

> Tested on arm qemu without regression.
>
> OK for trunk?
>
> thanks
> Carrot
>
> 2012-05-28 ?Wei Guozhi ?<carrot@google.com>
>
> ? ? ? ?PR target/53447
> ? ? ? ?* gcc.target/arm/pr53447-2.c: New testcase.
>
>
> 2012-05-28 ?Wei Guozhi ?<carrot@google.com>
>
> ? ? ? ?PR target/53447
> ? ? ? ?* config/arm/arm-protos.h (const_ok_for_anddi): New prototype.
> ? ? ? ?* config/arm/arm.c (const_ok_for_anddi): New function.
> ? ? ? ?* config/arm/constraints.md (De): New constraint.
> ? ? ? ?* config/arm/predicates.md (arm_anddi_operand): New predicate.
> ? ? ? ?(arm_immediate_anddi_operand): Likewise.
> ? ? ? ?(anddi_operand): Likewise.
> ? ? ? ?* config/arm/arm.md (anddi3): Extend it to handle 64bit constants.
> ? ? ? ?(anddi3_insn): Likewise.
> ? ? ? ?* config/arm/neon.md (anddi3_neon): Likewise.
>
>
>
> Index: testsuite/gcc.target/arm/pr53447-2.c
> ===================================================================
> --- testsuite/gcc.target/arm/pr53447-2.c ? ? ? ?(revision 0)
> +++ testsuite/gcc.target/arm/pr53447-2.c ? ? ? ?(revision 0)
> @@ -0,0 +1,8 @@
> +/* { dg-options "-O2" } ?*/
> +/* { dg-require-effective-target arm32 } */
> +/* { dg-final { scan-assembler-not "mov" } } */
> +
> +void t0p(long long * p)
> +{
> + ?*p &= 0x100000002;
> +}
> Index: config/arm/arm.c
> ===================================================================
> --- config/arm/arm.c ? ?(revision 187927)
> +++ config/arm/arm.c ? ?(working copy)
> @@ -2497,6 +2497,18 @@
> ? ? }
> ?}
>
> +/* Return TRUE if int I is a valid immediate constant used by pattern
> + ? anddi3_insn. ?*/
> +int
> +const_ok_for_anddi (HOST_WIDE_INT i)
> +{
> + ?HOST_WIDE_INT high = ARM_SIGN_EXTEND ((i >> 32) & 0xFFFFFFFF);
> + ?HOST_WIDE_INT low = ARM_SIGN_EXTEND (i & 0xFFFFFFFF);
> +
> + ?return (TARGET_32BIT && (const_ok_for_arm (low) || const_ok_for_arm (~low))
> + ? ? ? ? && (const_ok_for_arm (high) || const_ok_for_arm (~high)));
> +}
> +
> ?/* Emit a sequence of insns to handle a large constant.
> ? ?CODE is the code of the operation required, it can be any of SET, PLUS,
> ? ?IOR, AND, XOR, MINUS;
> Index: config/arm/arm-protos.h
> ===================================================================
> --- config/arm/arm-protos.h ? ? (revision 187927)
> +++ config/arm/arm-protos.h ? ? (working copy)
> @@ -47,6 +47,7 @@
> ?extern bool arm_small_register_classes_for_mode_p (enum machine_mode);
> ?extern int arm_hard_regno_mode_ok (unsigned int, enum machine_mode);
> ?extern bool arm_modes_tieable_p (enum machine_mode, enum machine_mode);
> +extern int const_ok_for_anddi (HOST_WIDE_INT);
> ?extern int const_ok_for_arm (HOST_WIDE_INT);
> ?extern int const_ok_for_op (HOST_WIDE_INT, enum rtx_code);
> ?extern int arm_split_constant (RTX_CODE, enum machine_mode, rtx,
> Index: config/arm/neon.md
> ===================================================================
> --- config/arm/neon.md ?(revision 187927)
> +++ config/arm/neon.md ?(working copy)
> @@ -774,9 +774,9 @@
> ?)
>
> ?(define_insn "anddi3_neon"
> - ?[(set (match_operand:DI 0 "s_register_operand" "=w,w,?&r,?&r,?w,?w")
> - ? ? ? ?(and:DI (match_operand:DI 1 "s_register_operand" "%w,0,0,r,w,0")
> - ? ? ? ? ? ? ? (match_operand:DI 2 "neon_inv_logic_op2" "w,DL,r,r,w,DL")))]
> + ?[(set (match_operand:DI 0 "s_register_operand" "=w,w,?&r,?&r,?w,?w,?&r,?&r")
> + ? ? ? ?(and:DI (match_operand:DI 1 "s_register_operand" "%w,0,0,r,w,0,0,r")
> + ? ? ? ? ? ? ? (match_operand:DI 2 "anddi_operand" "w,DL,r,r,w,DL,De,De")))]
> ? "TARGET_NEON"
> ?{
> ? switch (which_alternative)
> @@ -788,12 +788,14 @@
> ? ? ? ? ? ? ? ? ? ? DImode, 1, VALID_NEON_QREG_MODE (DImode));
> ? ? case 2: return "#";
> ? ? case 3: return "#";
> + ? ?case 6: return "#";
> + ? ?case 7: return "#";
> ? ? default: gcc_unreachable ();
> ? ? }
> ?}
> - ?[(set_attr "neon_type" "neon_int_1,neon_int_1,*,*,neon_int_1,neon_int_1")
> - ? (set_attr "length" "*,*,8,8,*,*")
> - ? (set_attr "arch" "nota8,nota8,*,*,onlya8,onlya8")]
> + ?[(set_attr "neon_type" "neon_int_1,neon_int_1,*,*,neon_int_1,neon_int_1,*,*")
> + ? (set_attr "length" "*,*,8,8,*,*,8,8")
> + ? (set_attr "arch" "nota8,nota8,*,*,onlya8,onlya8,*,*")]
> ?)
>
> ?(define_insn "orn<mode>3_neon"
> Index: config/arm/constraints.md
> ===================================================================
> --- config/arm/constraints.md ? (revision 187927)
> +++ config/arm/constraints.md ? (working copy)
> @@ -29,7 +29,7 @@
> ?;; in Thumb-1 state: I, J, K, L, M, N, O
>
> ?;; The following multi-letter normal constraints have been used:
> -;; in ARM/Thumb-2 state: Da, Db, Dc, Dn, Dl, DL, Dv, Dy, Di, Dt, Dz
> +;; in ARM/Thumb-2 state: Da, Db, Dc, De, Dn, Dl, DL, Dv, Dy, Di, Dt, Dz
> ?;; in Thumb-1 state: Pa, Pb, Pc, Pd, Pe
> ?;; in Thumb-2 state: Pj, PJ, Ps, Pt, Pu, Pv, Pw, Px, Py
>
> @@ -251,6 +251,12 @@
> ? ? ? (match_test "TARGET_32BIT && arm_const_double_inline_cost (op) == 4
> ? ? ? ? ? ? ? ? ? && !(optimize_size || arm_ld_sched)")))
>
> +(define_constraint "De"
> + "@internal
> + ?In ARM/Thumb-2 state a const_int that can be used by anddi3_insn. ?"
> + (and (match_code "const_int")
> + ? ? ?(match_test "TARGET_32BIT && const_ok_for_anddi (ival)")))
> +
> ?(define_constraint "Di"
> ?"@internal
> ? In ARM/Thumb-2 state a const_int or const_double where both the high
> Index: config/arm/predicates.md
> ===================================================================
> --- config/arm/predicates.md ? ?(revision 187927)
> +++ config/arm/predicates.md ? ?(working copy)
> @@ -117,6 +117,14 @@
> ? (and (match_code "const_int,const_double")
> ? ? ? ?(match_test "arm_const_double_by_immediates (op)")))
>
> +(define_predicate "arm_immediate_anddi_operand"
> + ?(and (match_code "const_int")
> + ? ? ? (match_test "const_ok_for_anddi (INTVAL (op))")))
> +
> +(define_predicate "arm_anddi_operand"
> + ?(ior (match_operand 0 "arm_immediate_anddi_operand")
> + ? ? ? (match_operand 0 "s_register_operand")))
> +
> ?(define_predicate "arm_neg_immediate_operand"
> ? (and (match_code "const_int")
> ? ? ? ?(match_test "const_ok_for_arm (-INTVAL (op))")))
> @@ -551,6 +559,10 @@
> ? (ior (match_operand 0 "imm_for_neon_inv_logic_operand")
> ? ? ? ?(match_operand 0 "s_register_operand")))
>
> +(define_predicate "anddi_operand"
> + ?(ior (match_operand 0 "neon_inv_logic_op2")
> + ? ? ? (match_operand 0 "arm_anddi_operand")))
> +
> ?;; TODO: We could check lane numbers more precisely based on the mode.
> ?(define_predicate "neon_lane_number"
> ? (and (match_code "const_int")
> Index: config/arm/arm.md
> ===================================================================
> --- config/arm/arm.md ? (revision 187927)
> +++ config/arm/arm.md ? (working copy)
> @@ -2072,17 +2072,38 @@
> ?(define_expand "anddi3"
> ? [(set (match_operand:DI ? ? ? ? 0 "s_register_operand" "")
> ? ? ? ?(and:DI (match_operand:DI 1 "s_register_operand" "")
> - ? ? ? ? ? ? ? (match_operand:DI 2 "neon_inv_logic_op2" "")))]
> + ? ? ? ? ? ? ? (match_operand:DI 2 "anddi_operand" "")))]
> ? "TARGET_32BIT"
> ? ""
> ?)
>
> -(define_insn "*anddi3_insn"
> - ?[(set (match_operand:DI ? ? ? ? 0 "s_register_operand" "=&r,&r")
> - ? ? ? (and:DI (match_operand:DI 1 "s_register_operand" ?"%0,r")
> - ? ? ? ? ? ? ? (match_operand:DI 2 "s_register_operand" ? "r,r")))]
> +(define_insn_and_split "*anddi3_insn"
> + ?[(set (match_operand:DI ? ? ? ? 0 "s_register_operand" "=&r,&r,&r,&r")
> + ? ? ? (and:DI (match_operand:DI 1 "s_register_operand" ?"%0,r,0,r")
> + ? ? ? ? ? ? ? (match_operand:DI 2 "arm_anddi_operand" ? "r,r,De,De")))]
> ? "TARGET_32BIT && !TARGET_IWMMXT && !TARGET_NEON"
> ? "#"
> + ?"TARGET_32BIT && !TARGET_IWMMXT && reload_completed"
> + ?[(set (match_dup 0) (and:SI (match_dup 1) (match_dup 2)))
> + ? (set (match_dup 3) (and:SI (match_dup 4) (match_dup 5)))]
> + ?"
> + ?{
> + ? ?operands[3] = gen_highpart (SImode, operands[0]);
> + ? ?operands[0] = gen_lowpart (SImode, operands[0]);
> + ? ?operands[4] = gen_highpart (SImode, operands[1]);
> + ? ?operands[1] = gen_lowpart (SImode, operands[1]);
> + ? ?if (GET_CODE (operands[2]) == CONST_INT)
> + ? ? ?{
> + ? ? ? HOST_WIDE_INT v = INTVAL (operands[2]);
> + ? ? ? operands[5] = GEN_INT (ARM_SIGN_EXTEND ((v >> 32) & 0xFFFFFFFF));
> + ? ? ? operands[2] = GEN_INT (ARM_SIGN_EXTEND (v & 0xFFFFFFFF));
> + ? ? ?}
> + ? ?else
> + ? ? ?{
> + ? ? ? operands[5] = gen_highpart (SImode, operands[2]);
> + ? ? ? operands[2] = gen_lowpart (SImode, operands[2]);
> + ? ? ?}
> + ?}"
> ? [(set_attr "length" "8")]
> ?)