| Message ID | CAEe8uEDfMSaYnFgFUJPRdzXK-VBL=kn5T7z7x7nNui-pmPgT7Q@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On 29 June 2012 12:23, Carrot Wei <carrot@google.com> wrote: > Hi > > So the following is updated patch. Tested on qemu with arm/thumb modes Assuming this testing was with and without neon ? Because the patterns changed are different whether you use Neon or not. > without regression. Can you add some tests for all 4 cases ? See comments inline below for some changes ? Ok with those changes if no regressions for above mentioned testing. > > thanks > Carrot > > > 2012-06-29 Wei Guozhi <carrot@google.com> > > PR target/53447 > * gcc.target/arm/pr53447-1.c: New testcase. > * gcc.target/arm/pr53447-2.c: New testcase. > > > 2012-06-29 Wei Guozhi <carrot@google.com> > > PR target/53447 > * config/arm/arm-protos.h (const_ok_for_dimode_op): New prototype. > * config/arm/arm.c (const_ok_for_dimode_op): New function. > * config/arm/constraints.md (Dd): New constraint. > * config/arm/predicates.md (arm_adddi_operand): New predicate. > * config/arm/arm.md (adddi3): Extend it to handle constants. > (arm_adddi3): Likewise. > (addsi3_carryin_<optab>): Extend it to handle sbc case. > * config/arm/neon.md (adddi3_neon): Extend it to handle constants. > > > Index: testsuite/gcc.target/arm/pr53447-1.c > =================================================================== > --- testsuite/gcc.target/arm/pr53447-1.c (revision 0) > +++ testsuite/gcc.target/arm/pr53447-1.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 += 0x100000001; > +} > 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 -= 0x100000008; > +} > Index: config/arm/arm.c > =================================================================== > --- config/arm/arm.c (revision 187751) > +++ config/arm/arm.c (working copy) > @@ -2497,6 +2497,28 @@ > } > } > > +/* Return true if I is a valid di mode constant for the operation CODE. */ > +int > +const_ok_for_dimode_op (HOST_WIDE_INT i, enum rtx_code code) > +{ > + HOST_WIDE_INT hi_val = (i >> 32) & 0xFFFFFFFF; > + HOST_WIDE_INT lo_val = i & 0xFFFFFFFF; > + rtx hi = GEN_INT (hi_val); > + rtx lo = GEN_INT (lo_val); > + > + if (TARGET_THUMB1) > + return 0; > + > + switch (code) > + { > + case PLUS: > + return arm_not_operand (hi, SImode) && arm_add_operand (lo, SImode); > + > + default: > + return 0; > + } > +} > + > /* 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 187751) > +++ config/arm/arm-protos.h (working copy) > @@ -49,6 +49,7 @@ > extern bool arm_modes_tieable_p (enum machine_mode, enum machine_mode); > extern int const_ok_for_arm (HOST_WIDE_INT); > extern int const_ok_for_op (HOST_WIDE_INT, enum rtx_code); > +extern int const_ok_for_dimode_op (HOST_WIDE_INT, enum rtx_code); > extern int arm_split_constant (RTX_CODE, enum machine_mode, rtx, > HOST_WIDE_INT, rtx, rtx, int); > extern RTX_CODE arm_canonicalize_comparison (RTX_CODE, rtx *, rtx *); > Index: config/arm/neon.md > =================================================================== > --- config/arm/neon.md (revision 187751) > +++ config/arm/neon.md (working copy) > @@ -588,9 +588,9 @@ > ) > > (define_insn "adddi3_neon" > - [(set (match_operand:DI 0 "s_register_operand" "=w,?&r,?&r,?w") > - (plus:DI (match_operand:DI 1 "s_register_operand" "%w,0,0,w") > - (match_operand:DI 2 "s_register_operand" "w,r,0,w"))) > + [(set (match_operand:DI 0 "s_register_operand" "=w,?&r,?&r,?w,?&r,?&r,?&r") > + (plus:DI (match_operand:DI 1 "s_register_operand" "%w,0,0,w,r,0,r") > + (match_operand:DI 2 "arm_adddi_operand" > "w,r,0,w,r,Dd,Dd"))) > (clobber (reg:CC CC_REGNUM))] > "TARGET_NEON" > { > @@ -600,13 +600,16 @@ > case 3: return "vadd.i64\t%P0, %P1, %P2"; > case 1: return "#"; > case 2: return "#"; > + case 4: return "#"; > + case 5: return "#"; > + case 6: return "#"; > default: gcc_unreachable (); > } > } > - [(set_attr "neon_type" "neon_int_1,*,*,neon_int_1") > - (set_attr "conds" "*,clob,clob,*") > - (set_attr "length" "*,8,8,*") > - (set_attr "arch" "nota8,*,*,onlya8")] > + [(set_attr "neon_type" "neon_int_1,*,*,neon_int_1,*,*,*") > + (set_attr "conds" "*,clob,clob,*,clob,clob,clob") > + (set_attr "length" "*,8,8,*,8,8,8") > + (set_attr "arch" "nota8,*,*,onlya8,*,*,*")] > ) > > (define_insn "*sub<mode>3_neon" > Index: config/arm/constraints.md > =================================================================== > --- config/arm/constraints.md (revision 187751) > +++ 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, Dd, 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 "Dd" > + "@internal > + In ARM/Thumb-2 state a const_int that can be used by insn adddi." > + (and (match_code "const_int") > + (match_test "TARGET_32BIT && const_ok_for_dimode_op (ival, PLUS)"))) > + > (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 187751) > +++ config/arm/predicates.md (working copy) > @@ -154,6 +154,11 @@ > (ior (match_operand 0 "arm_rhs_operand") > (match_operand 0 "arm_neg_immediate_operand"))) > > +(define_predicate "arm_adddi_operand" > + (ior (match_operand 0 "s_register_operand") > + (and (match_code "const_int") > + (match_test "const_ok_for_dimode_op (INTVAL (op), PLUS)")))) > + > (define_predicate "arm_addimm_operand" > (ior (match_operand 0 "arm_immediate_operand") > (match_operand 0 "arm_neg_immediate_operand"))) > Index: config/arm/arm.md > =================================================================== > --- config/arm/arm.md (revision 187751) > +++ config/arm/arm.md (working copy) > @@ -574,7 +574,7 @@ > [(parallel > [(set (match_operand:DI 0 "s_register_operand" "") > (plus:DI (match_operand:DI 1 "s_register_operand" "") > - (match_operand:DI 2 "s_register_operand" ""))) > + (match_operand:DI 2 "arm_adddi_operand" ""))) > (clobber (reg:CC CC_REGNUM))])] > "TARGET_EITHER" > " > @@ -609,10 +609,10 @@ > [(set_attr "length" "4")] > ) > > -(define_insn_and_split "*arm_adddi3" > - [(set (match_operand:DI 0 "s_register_operand" "=&r,&r") > - (plus:DI (match_operand:DI 1 "s_register_operand" "%0, 0") > - (match_operand:DI 2 "s_register_operand" "r, 0"))) > +(define_insn_and_split "arm_adddi3" > + [(set (match_operand:DI 0 "s_register_operand" "=&r,&r,&r,&r,&r") > + (plus:DI (match_operand:DI 1 "s_register_operand" "%0, 0, r, 0, r") > + (match_operand:DI 2 "arm_adddi_operand" "r, 0, r, Dd, Dd"))) > (clobber (reg:CC CC_REGNUM))] > "TARGET_32BIT && !(TARGET_HARD_FLOAT && TARGET_MAVERICK) && !TARGET_NEON" > "#" > @@ -630,8 +630,17 @@ > operands[0] = gen_lowpart (SImode, operands[0]); > operands[4] = gen_highpart (SImode, operands[1]); > operands[1] = gen_lowpart (SImode, operands[1]); > - operands[5] = gen_highpart (SImode, operands[2]); > - operands[2] = gen_lowpart (SImode, operands[2]); > + 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]); > + } Instead operands[5] = gen_highpart_mode (SImode, DImode, operands[2]); operands[2] = gen_lowpart (SImode, operands[2]); So you don't need a check there. > }" > [(set_attr "conds" "clob") > (set_attr "length" "8")] > @@ -980,12 +989,14 @@ > ) > > (define_insn "*addsi3_carryin_<optab>" > - [(set (match_operand:SI 0 "s_register_operand" "=r") > - (plus:SI (plus:SI (match_operand:SI 1 "s_register_operand" "%r") > - (match_operand:SI 2 "arm_rhs_operand" "rI")) > + [(set (match_operand:SI 0 "s_register_operand" "=r,r") > + (plus:SI (plus:SI (match_operand:SI 1 "s_register_operand" "%r,r") > + (match_operand:SI 2 "arm_not_operand" "rI,K")) > (LTUGEU:SI (reg:<cnb> CC_REGNUM) (const_int 0))))] > "TARGET_32BIT" > - "adc%?\\t%0, %1, %2" > + "@ > + adc%?\\t%0, %1, %2 > + sbc%?\\t%0, %1, #%B2" > [(set_attr "conds" "use")] > ) Any reason why you didn't consider making these changes to the *addsi3_carryin_alt2<optab> pattern ? regards, Ramana > > > > > On Thu, Jun 28, 2012 at 7:48 PM, Ramana Radhakrishnan > <ramana.radhakrishnan@linaro.org> wrote: >> >> > subs -lo ; sbc ~hi - lower negative, upper negative >> > subs -lo ; adc hi - lower negative, upper positive >> >> Yes. >> >> <snip> >> >> >> >> >>> >> >>>> (LTUGEU:SI (reg:<cnb> CC_REGNUM) (const_int 0))))] >> >>>> "TARGET_32BIT" >> >>>> - "adc%?\\t%0, %1, %2" >> >>>> + "@ >> >>>> + adc%?\\t%0, %1, %2 >> >>>> + sbc%?\\t%0, %1, %#n2" >> > >> > Since constraint "K" is logical not, not negative, should the last >> > line be following? >> > >> > + sbc%?\\t%0, %1, #%B2" >> >> Indeed that was a typo on my part. Sorry about that. >> >> Ramana
Index: testsuite/gcc.target/arm/pr53447-1.c =================================================================== --- testsuite/gcc.target/arm/pr53447-1.c (revision 0) +++ testsuite/gcc.target/arm/pr53447-1.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 += 0x100000001; +} 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 -= 0x100000008; +} Index: config/arm/arm.c =================================================================== --- config/arm/arm.c (revision 187751) +++ config/arm/arm.c (working copy) @@ -2497,6 +2497,28 @@ } } +/* Return true if I is a valid di mode constant for the operation CODE. */ +int +const_ok_for_dimode_op (HOST_WIDE_INT i, enum rtx_code code) +{ + HOST_WIDE_INT hi_val = (i >> 32) & 0xFFFFFFFF; + HOST_WIDE_INT lo_val = i & 0xFFFFFFFF; + rtx hi = GEN_INT (hi_val); + rtx lo = GEN_INT (lo_val); + + if (TARGET_THUMB1) + return 0; + + switch (code) + { + case PLUS: + return arm_not_operand (hi, SImode) && arm_add_operand (lo, SImode); + + default: + return 0; + } +} + /* 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 187751) +++ config/arm/arm-protos.h (working copy) @@ -49,6 +49,7 @@ extern bool arm_modes_tieable_p (enum machine_mode, enum machine_mode); extern int const_ok_for_arm (HOST_WIDE_INT); extern int const_ok_for_op (HOST_WIDE_INT, enum rtx_code); +extern int const_ok_for_dimode_op (HOST_WIDE_INT, enum rtx_code); extern int arm_split_constant (RTX_CODE, enum machine_mode, rtx, HOST_WIDE_INT, rtx, rtx, int); extern RTX_CODE arm_canonicalize_comparison (RTX_CODE, rtx *, rtx *); Index: config/arm/neon.md =================================================================== --- config/arm/neon.md (revision 187751) +++ config/arm/neon.md (working copy) @@ -588,9 +588,9 @@ ) (define_insn "adddi3_neon" - [(set (match_operand:DI 0 "s_register_operand" "=w,?&r,?&r,?w") - (plus:DI (match_operand:DI 1 "s_register_operand" "%w,0,0,w") - (match_operand:DI 2 "s_register_operand" "w,r,0,w"))) + [(set (match_operand:DI 0 "s_register_operand" "=w,?&r,?&r,?w,?&r,?&r,?&r") + (plus:DI (match_operand:DI 1 "s_register_operand" "%w,0,0,w,r,0,r") + (match_operand:DI 2 "arm_adddi_operand" "w,r,0,w,r,Dd,Dd"))) (clobber (reg:CC CC_REGNUM))] "TARGET_NEON" { @@ -600,13 +600,16 @@ case 3: return "vadd.i64\t%P0, %P1, %P2"; case 1: return "#"; case 2: return "#"; + case 4: return "#"; + case 5: return "#"; + case 6: return "#"; default: gcc_unreachable (); } } - [(set_attr "neon_type" "neon_int_1,*,*,neon_int_1") - (set_attr "conds" "*,clob,clob,*") - (set_attr "length" "*,8,8,*") - (set_attr "arch" "nota8,*,*,onlya8")] + [(set_attr "neon_type" "neon_int_1,*,*,neon_int_1,*,*,*") + (set_attr "conds" "*,clob,clob,*,clob,clob,clob") + (set_attr "length" "*,8,8,*,8,8,8") + (set_attr "arch" "nota8,*,*,onlya8,*,*,*")] ) (define_insn "*sub<mode>3_neon" Index: config/arm/constraints.md =================================================================== --- config/arm/constraints.md (revision 187751) +++ 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, Dd, 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 "Dd" + "@internal + In ARM/Thumb-2 state a const_int that can be used by insn adddi." + (and (match_code "const_int") + (match_test "TARGET_32BIT && const_ok_for_dimode_op (ival, PLUS)"))) + (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 187751) +++ config/arm/predicates.md (working copy) @@ -154,6 +154,11 @@ (ior (match_operand 0 "arm_rhs_operand") (match_operand 0 "arm_neg_immediate_operand"))) +(define_predicate "arm_adddi_operand" + (ior (match_operand 0 "s_register_operand") + (and (match_code "const_int") + (match_test "const_ok_for_dimode_op (INTVAL (op), PLUS)")))) + (define_predicate "arm_addimm_operand" (ior (match_operand 0 "arm_immediate_operand") (match_operand 0 "arm_neg_immediate_operand"))) Index: config/arm/arm.md =================================================================== --- config/arm/arm.md (revision 187751) +++ config/arm/arm.md (working copy) @@ -574,7 +574,7 @@ [(parallel [(set (match_operand:DI 0 "s_register_operand" "") (plus:DI (match_operand:DI 1 "s_register_operand" "") - (match_operand:DI 2 "s_register_operand" ""))) + (match_operand:DI 2 "arm_adddi_operand" ""))) (clobber (reg:CC CC_REGNUM))])] "TARGET_EITHER" " @@ -609,10 +609,10 @@ [(set_attr "length" "4")] ) -(define_insn_and_split "*arm_adddi3" - [(set (match_operand:DI 0 "s_register_operand" "=&r,&r") - (plus:DI (match_operand:DI 1 "s_register_operand" "%0, 0") - (match_operand:DI 2 "s_register_operand" "r, 0"))) +(define_insn_and_split "arm_adddi3" + [(set (match_operand:DI 0 "s_register_operand" "=&r,&r,&r,&r,&r") + (plus:DI (match_operand:DI 1 "s_register_operand" "%0, 0, r, 0, r") + (match_operand:DI 2 "arm_adddi_operand" "r, 0, r, Dd, Dd"))) (clobber (reg:CC CC_REGNUM))] "TARGET_32BIT && !(TARGET_HARD_FLOAT && TARGET_MAVERICK) && !TARGET_NEON" "#" @@ -630,8 +630,17 @@ operands[0] = gen_lowpart (SImode, operands[0]); operands[4] = gen_highpart (SImode, operands[1]); operands[1] = gen_lowpart (SImode, operands[1]); - operands[5] = gen_highpart (SImode, operands[2]); - operands[2] = gen_lowpart (SImode, operands[2]); + 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 "conds" "clob") (set_attr "length" "8")] @@ -980,12 +989,14 @@ ) (define_insn "*addsi3_carryin_<optab>" - [(set (match_operand:SI 0 "s_register_operand" "=r") - (plus:SI (plus:SI (match_operand:SI 1 "s_register_operand" "%r") - (match_operand:SI 2 "arm_rhs_operand" "rI")) + [(set (match_operand:SI 0 "s_register_operand" "=r,r") + (plus:SI (plus:SI (match_operand:SI 1 "s_register_operand" "%r,r") + (match_operand:SI 2 "arm_not_operand" "rI,K")) (LTUGEU:SI (reg:<cnb> CC_REGNUM) (const_int 0))))] "TARGET_32BIT" - "adc%?\\t%0, %1, %2" + "@ + adc%?\\t%0, %1, %2 + sbc%?\\t%0, %1, #%B2" [(set_attr "conds" "use")] )