| Message ID | 7794A52CE4D579448B959EED7DD0A4723DD0973D@satlexdag06.amd.com |
|---|---|
| State | New |
| Headers | show |
On Mon, May 25, 2015 at 06:39:36AM +0100, Kumar, Venkataramanan wrote: > Ping! > > -----Original Message----- > From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-owner@gcc.gnu.org] On Behalf Of Kumar, Venkataramanan > Sent: Tuesday, May 19, 2015 9:07 PM > To: James Greenhalgh (james.greenhalgh@arm.com); gcc-patches@gcc.gnu.org > Cc: Kyrill Tkachov (kyrylo.tkachov@arm.com); ramana.radhakrishnan@arm.com; segher@kernel.crashing.org; Marcus Shawcroft (marcus.shawcroft@arm.com) > Subject: [Patch] [AArch64] PR target 66049: fix add/extend gcc test suite failures > > Hi Maintainers, > > Please find the attached patch, that fixes add/extend gcc test suite failures in Aarch64 target. > Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66049 > > These tests started to fail after we prevented combiner from converting shift > RTX to mult RTX, when the RTX is not inside a memory operation (r222874) . > Now I have added new add/extend patterns which are based on shift operations, > to fix these cases. > > Testing status with the patch. > > (1) GCC bootstrap on AArch64 successful. > (2) SPEC2006 INT runs did not show any degradation. > (3) gcc regression testing passed. Perfect, thank you. > Is this OK for trunk ? Yup, this is OK for trunk. Make sure you reference the PR number in the ChangeLog entry and close off the BZ entry when this is done. Thanks again for your patience. > +;; zero_extend version of above > +(define_insn "*add_uxtsi_shift2_uxtw" > + [(set (match_operand:DI 0 "register_operand" "=rk") > + (zero_extend:DI > + (plus:SI (and:SI > + (ashift:SI (match_operand:SI 1 "register_operand" "r") > + (match_operand 2 "aarch64_imm3" "Ui3")) > + (match_operand 3 "const_int_operand" "n")) > + (match_operand:SI 4 "register_operand" "r"))))] > + "aarch64_uxt_size (INTVAL (operands[2]), INTVAL (operands[3])) != 0" > + "* > + operands[3] = GEN_INT (aarch64_uxt_size (INTVAL (operands[2]), > + INTVAL (operands[3]))); > + return \"add\t%w0, %w4, %w1, uxt%e3 %2\";" > + [(set_attr "type" "alu_ext")] > +) > + You don't have to fix it in this patch (as it matches existing style), but if you are looking for a cleanup, we should use the { /* Code */ } syntax in these patterns and avoid all the escaping of '"'. Cheers, James
Thanks James Committed as https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=223703 Regards, Venkat. > -----Original Message----- > From: James Greenhalgh [mailto:james.greenhalgh@arm.com] > Sent: Tuesday, May 26, 2015 2:09 PM > To: Kumar, Venkataramanan > Cc: gcc-patches@gcc.gnu.org; Marcus Shawcroft > Subject: Re: [Ping]: [Patch] [AArch64] PR target 66049: fix add/extend gcc > test suite failures > > On Mon, May 25, 2015 at 06:39:36AM +0100, Kumar, Venkataramanan wrote: > > Ping! > > > > -----Original Message----- > > From: gcc-patches-owner@gcc.gnu.org > > [mailto:gcc-patches-owner@gcc.gnu.org] On Behalf Of Kumar, > > Venkataramanan > > Sent: Tuesday, May 19, 2015 9:07 PM > > To: James Greenhalgh (james.greenhalgh@arm.com); > > gcc-patches@gcc.gnu.org > > Cc: Kyrill Tkachov (kyrylo.tkachov@arm.com); > > ramana.radhakrishnan@arm.com; segher@kernel.crashing.org; Marcus > > Shawcroft (marcus.shawcroft@arm.com) > > Subject: [Patch] [AArch64] PR target 66049: fix add/extend gcc test > > suite failures > > > > Hi Maintainers, > > > > Please find the attached patch, that fixes add/extend gcc test suite failures > in Aarch64 target. > > Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66049 > > > > These tests started to fail after we prevented combiner from > > converting shift RTX to mult RTX, when the RTX is not inside a memory > operation (r222874) . > > Now I have added new add/extend patterns which are based on shift > > operations, to fix these cases. > > > > Testing status with the patch. > > > > (1) GCC bootstrap on AArch64 successful. > > (2) SPEC2006 INT runs did not show any degradation. > > (3) gcc regression testing passed. > > Perfect, thank you. > > > Is this OK for trunk ? > > Yup, this is OK for trunk. Make sure you reference the PR number in the > ChangeLog entry and close off the BZ entry when this is done. > > Thanks again for your patience. > > > +;; zero_extend version of above > > +(define_insn "*add_uxtsi_shift2_uxtw" > > + [(set (match_operand:DI 0 "register_operand" "=rk") > > + (zero_extend:DI > > + (plus:SI (and:SI > > + (ashift:SI (match_operand:SI 1 "register_operand" "r") > > + (match_operand 2 "aarch64_imm3" "Ui3")) > > + (match_operand 3 "const_int_operand" "n")) > > + (match_operand:SI 4 "register_operand" "r"))))] > > + "aarch64_uxt_size (INTVAL (operands[2]), INTVAL (operands[3])) != 0" > > + "* > > + operands[3] = GEN_INT (aarch64_uxt_size (INTVAL (operands[2]), > > + INTVAL (operands[3]))); > > + return \"add\t%w0, %w4, %w1, uxt%e3 %2\";" > > + [(set_attr "type" "alu_ext")] > > +) > > + > > You don't have to fix it in this patch (as it matches existing style), but if you > are looking for a cleanup, we should use the > > { > /* Code */ > } > > syntax in these patterns and avoid all the escaping of '"'. > > Cheers, > James
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md index 1dbadc0..d0d6a6a 100644 --- a/gcc/config/aarch64/aarch64.md +++ b/gcc/config/aarch64/aarch64.md @@ -1539,6 +1539,38 @@ [(set_attr "type" "alus_sreg,alus_imm,alus_imm")] ) +(define_insn "*adds_shift_imm_<mode>" + [(set (reg:CC_NZ CC_REGNUM) + (compare:CC_NZ + (plus:GPI (ASHIFT:GPI + (match_operand:GPI 1 "register_operand" "r") + (match_operand:QI 2 "aarch64_shift_imm_<mode>" "n")) + (match_operand:GPI 3 "register_operand" "r")) + (const_int 0))) + (set (match_operand:GPI 0 "register_operand" "=r") + (plus:GPI (ASHIFT:GPI (match_dup 1) (match_dup 2)) + (match_dup 3)))] + "" + "adds\\t%<w>0, %<w>3, %<w>1, <shift> %2" + [(set_attr "type" "alus_shift_imm")] +) + +(define_insn "*subs_shift_imm_<mode>" + [(set (reg:CC_NZ CC_REGNUM) + (compare:CC_NZ + (minus:GPI (match_operand:GPI 1 "register_operand" "r") + (ASHIFT:GPI + (match_operand:GPI 2 "register_operand" "r") + (match_operand:QI 3 "aarch64_shift_imm_<mode>" "n"))) + (const_int 0))) + (set (match_operand:GPI 0 "register_operand" "=r") + (minus:GPI (match_dup 1) + (ASHIFT:GPI (match_dup 2) (match_dup 3))))] + "" + "subs\\t%<w>0, %<w>1, %<w>2, <shift> %3" + [(set_attr "type" "alus_shift_imm")] +) + (define_insn "*adds_mul_imm_<mode>" [(set (reg:CC_NZ CC_REGNUM) (compare:CC_NZ @@ -1599,6 +1631,42 @@ [(set_attr "type" "alus_ext")] ) +(define_insn "*adds_<optab><ALLX:mode>_shift_<GPI:mode>" + [(set (reg:CC_NZ CC_REGNUM) + (compare:CC_NZ + (plus:GPI (ashift:GPI + (ANY_EXTEND:GPI + (match_operand:ALLX 1 "register_operand" "r")) + (match_operand 2 "aarch64_imm3" "Ui3")) + (match_operand:GPI 3 "register_operand" "r")) + (const_int 0))) + (set (match_operand:GPI 0 "register_operand" "=rk") + (plus:GPI (ashift:GPI (ANY_EXTEND:GPI (match_dup 1)) + (match_dup 2)) + (match_dup 3)))] + "" + "adds\\t%<GPI:w>0, %<GPI:w>3, %<GPI:w>1, <su>xt<ALLX:size> %2" + [(set_attr "type" "alus_ext")] +) + +(define_insn "*subs_<optab><ALLX:mode>_shift_<GPI:mode>" + [(set (reg:CC_NZ CC_REGNUM) + (compare:CC_NZ + (minus:GPI (match_operand:GPI 1 "register_operand" "r") + (ashift:GPI + (ANY_EXTEND:GPI + (match_operand:ALLX 2 "register_operand" "r")) + (match_operand 3 "aarch64_imm3" "Ui3"))) + (const_int 0))) + (set (match_operand:GPI 0 "register_operand" "=rk") + (minus:GPI (match_dup 1) + (ashift:GPI (ANY_EXTEND:GPI (match_dup 2)) + (match_dup 3))))] + "" + "subs\\t%<GPI:w>0, %<GPI:w>1, %<GPI:w>2, <su>xt<ALLX:size> %3" + [(set_attr "type" "alus_ext")] +) + (define_insn "*adds_<optab><mode>_multp2" [(set (reg:CC_NZ CC_REGNUM) (compare:CC_NZ @@ -1894,6 +1962,38 @@ [(set_attr "type" "adc_reg")] ) +(define_insn "*add_uxt<mode>_shift2" + [(set (match_operand:GPI 0 "register_operand" "=rk") + (plus:GPI (and:GPI + (ashift:GPI (match_operand:GPI 1 "register_operand" "r") + (match_operand 2 "aarch64_imm3" "Ui3")) + (match_operand 3 "const_int_operand" "n")) + (match_operand:GPI 4 "register_operand" "r")))] + "aarch64_uxt_size (INTVAL (operands[2]), INTVAL (operands[3])) != 0" + "* + operands[3] = GEN_INT (aarch64_uxt_size (INTVAL(operands[2]), + INTVAL (operands[3]))); + return \"add\t%<w>0, %<w>4, %<w>1, uxt%e3 %2\";" + [(set_attr "type" "alu_ext")] +) + +;; zero_extend version of above +(define_insn "*add_uxtsi_shift2_uxtw" + [(set (match_operand:DI 0 "register_operand" "=rk") + (zero_extend:DI + (plus:SI (and:SI + (ashift:SI (match_operand:SI 1 "register_operand" "r") + (match_operand 2 "aarch64_imm3" "Ui3")) + (match_operand 3 "const_int_operand" "n")) + (match_operand:SI 4 "register_operand" "r"))))] + "aarch64_uxt_size (INTVAL (operands[2]), INTVAL (operands[3])) != 0" + "* + operands[3] = GEN_INT (aarch64_uxt_size (INTVAL (operands[2]), + INTVAL (operands[3]))); + return \"add\t%w0, %w4, %w1, uxt%e3 %2\";" + [(set_attr "type" "alu_ext")] +) + (define_insn "*add_uxt<mode>_multp2" [(set (match_operand:GPI 0 "register_operand" "=rk") (plus:GPI (and:GPI @@ -2150,6 +2250,38 @@ [(set_attr "type" "adc_reg")] ) +(define_insn "*sub_uxt<mode>_shift2" + [(set (match_operand:GPI 0 "register_operand" "=rk") + (minus:GPI (match_operand:GPI 4 "register_operand" "rk") + (and:GPI + (ashift:GPI (match_operand:GPI 1 "register_operand" "r") + (match_operand 2 "aarch64_imm3" "Ui3")) + (match_operand 3 "const_int_operand" "n"))))] + "aarch64_uxt_size (INTVAL (operands[2]),INTVAL (operands[3])) != 0" + "* + operands[3] = GEN_INT (aarch64_uxt_size (INTVAL (operands[2]), + INTVAL (operands[3]))); + return \"sub\t%<w>0, %<w>4, %<w>1, uxt%e3 %2\";" + [(set_attr "type" "alu_ext")] +) + +;; zero_extend version of above +(define_insn "*sub_uxtsi_shift2_uxtw" + [(set (match_operand:DI 0 "register_operand" "=rk") + (zero_extend:DI + (minus:SI (match_operand:SI 4 "register_operand" "rk") + (and:SI + (ashift:SI (match_operand:SI 1 "register_operand" "r") + (match_operand 2 "aarch64_imm3" "Ui3")) + (match_operand 3 "const_int_operand" "n")))))] + "aarch64_uxt_size (INTVAL (operands[2]),INTVAL (operands[3])) != 0" + "* + operands[3] = GEN_INT (aarch64_uxt_size (INTVAL (operands[2]), + INTVAL (operands[3]))); + return \"sub\t%w0, %w4, %w1, uxt%e3 %2\";" + [(set_attr "type" "alu_ext")] +) + (define_insn "*sub_uxt<mode>_multp2" [(set (match_operand:GPI 0 "register_operand" "=rk") (minus:GPI (match_operand:GPI 4 "register_operand" "rk")
Ping! -----Original Message----- From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-owner@gcc.gnu.org] On Behalf Of Kumar, Venkataramanan Sent: Tuesday, May 19, 2015 9:07 PM To: James Greenhalgh (james.greenhalgh@arm.com); gcc-patches@gcc.gnu.org Cc: Kyrill Tkachov (kyrylo.tkachov@arm.com); ramana.radhakrishnan@arm.com; segher@kernel.crashing.org; Marcus Shawcroft (marcus.shawcroft@arm.com) Subject: [Patch] [AArch64] PR target 66049: fix add/extend gcc test suite failures Hi Maintainers, Please find the attached patch, that fixes add/extend gcc test suite failures in Aarch64 target. Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66049 These tests started to fail after we prevented combiner from converting shift RTX to mult RTX, when the RTX is not inside a memory operation (r222874) . Now I have added new add/extend patterns which are based on shift operations, to fix these cases. Testing status with the patch. (1) GCC bootstrap on AArch64 successful. (2) SPEC2006 INT runs did not show any degradation. (3) gcc regression testing passed. (-----Snip-----) # Comparing 3 common sum files ## /bin/sh ./gcc-fsf-trunk/contrib/compare_tests /tmp/gxx-sum1.24998 /tmp/gxx-sum2.24998 Tests that now work, but didn't before: gcc.target/aarch64/adds1.c scan-assembler adds\tw[0-9]+, w[0-9]+, w[0-9]+, lsl 3 gcc.target/aarch64/adds1.c scan-assembler adds\tx[0-9]+, x[0-9]+, x[0-9]+, lsl 3 gcc.target/aarch64/adds3.c scan-assembler-times adds\tx[0-9]+, x[0-9]+, x[0-9]+, sxtw 2 gcc.target/aarch64/extend.c scan-assembler add\tw[0-9]+,.*uxth #?1 gcc.target/aarch64/extend.c scan-assembler add\tx[0-9]+,.*uxtw #?3 gcc.target/aarch64/extend.c scan-assembler sub\tw[0-9]+,.*uxth #?1 gcc.target/aarch64/extend.c scan-assembler sub\tx[0-9]+,.*uxth #?1 gcc.target/aarch64/extend.c scan-assembler sub\tx[0-9]+,.*uxtw #?3 gcc.target/aarch64/subs1.c scan-assembler subs\tw[0-9]+, w[0-9]+, w[0-9]+, lsl 3 gcc.target/aarch64/subs1.c scan-assembler subs\tx[0-9]+, x[0-9]+, x[0-9]+, lsl 3 gcc.target/aarch64/subs3.c scan-assembler-times subs\tx[0-9]+, x[0-9]+, x[0-9]+, sxtw 2 # No differences found in 3 common sum files (-----Snip-----) The patterns are fixing the regressing tests, so I have not added any new tests. Regarding removal of the old patterns based on "mults", I am planning to do it as a separate work. Is this OK for trunk ? gcc/ChangeLog 2015-05-19 Venkataramanan Kumar <venkataramanan.kumar@amd.com> * config/aarch64/aarch64.md (*adds_shift_imm_<mode>): New pattern. (*subs_shift_imm_<mode>): Likewise. (*adds_<optab><ALLX:mode>_shift_<GPI:mode>): Likewise. (*subs_<optab><ALLX:mode>_shift_<GPI:mode>): Likewise. (*add_uxt<mode>_shift2): Likewise. (*add_uxtsi_shift2_uxtw): Likewise. (*sub_uxt<mode>_shift2): Likewise. (*sub_uxtsi_shift2_uxtw): Likewise. Regards, Venkat.