| Message ID | 552D897C.2040207@linaro.org |
|---|---|
| State | New |
| Headers | show |
> On Apr 15, 2015, at 12:41 AM, Kugan <kugan.vivekanandarajah@linaro.org> wrote: > > This patch uses clobber with match_scratch instead of earlyclobber for > aarch64_lshr_sisd_or_int_<mode>3 so that RA can have more freedom in > selecting suitable register, as discussed in > http://thread.gmane.org/gmane.comp.gcc.patches/336162 and reported in > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65139. > > This is based on Maxim's patch. I have bootstrapped and regression > tested on aarch64-linux-gnu with no new regressions. Is this OK for trunk? > > Thanks, > Kugan > > gcc/ChangeLog: > > 2015-04-15 Kugan Vivekanandarajah <kuganv@linaro.org> > Maxim Kuvyrkov <maxim.kuvyrkov@linaro.org> > > PR target/65139 > * config/aarch64/aarch64.md (<optab><mode>3): Expand lshr with > gen_aarch64_lshr_sisd_or_int_<mode>3. > (*aarch64_lshr_sisd_or_int_<mode>3): Rename to > aarch64_lshr_sisd_or_int_<mode>3 and use clobber with > match_scratch instead of early clobber. > <p.txt> For the benefit of other reviewers -- I have reviewed this patch internally and it looks OK. Kugan, did you audit other patterns in aarch64.md to see if any other early-clobbers can be optimized in this way? Thank you, -- Maxim Kuvyrkov www.linaro.org
On 14/04/15 22:41, Kugan wrote: > This patch uses clobber with match_scratch instead of earlyclobber for > aarch64_lshr_sisd_or_int_<mode>3 so that RA can have more freedom in > selecting suitable register, as discussed in > http://thread.gmane.org/gmane.comp.gcc.patches/336162 and reported in > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65139. > > This is based on Maxim's patch. I have bootstrapped and regression > tested on aarch64-linux-gnu with no new regressions. Is this OK for trunk? > > Thanks, > Kugan > > gcc/ChangeLog: > > 2015-04-15 Kugan Vivekanandarajah <kuganv@linaro.org> > Maxim Kuvyrkov <maxim.kuvyrkov@linaro.org> > > PR target/65139 > * config/aarch64/aarch64.md (<optab><mode>3): Expand lshr with > gen_aarch64_lshr_sisd_or_int_<mode>3. > (*aarch64_lshr_sisd_or_int_<mode>3): Rename to > aarch64_lshr_sisd_or_int_<mode>3 and use clobber with > match_scratch instead of early clobber. > + if (strcmp ("<optab>", "lshr") == 0) + { This can't be the best way to match the operation type. Yes, I know that the comparison is a compile time invariant, but there must be an attribute of optab (or one can be created for it) that would make the test trivial and not rely on the compiler optimizing the strcmp away. R.
On Wed, Apr 15, 2015 at 01:18:36PM +0100, Richard Earnshaw wrote: > On 14/04/15 22:41, Kugan wrote: > >This patch uses clobber with match_scratch instead of earlyclobber for > >aarch64_lshr_sisd_or_int_<mode>3 so that RA can have more freedom in > >selecting suitable register, as discussed in > >http://thread.gmane.org/gmane.comp.gcc.patches/336162 and reported in > >https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65139. > > > >This is based on Maxim's patch. I have bootstrapped and regression > >tested on aarch64-linux-gnu with no new regressions. Is this OK for trunk? > > > >Thanks, > >Kugan > > > >gcc/ChangeLog: > > > >2015-04-15 Kugan Vivekanandarajah <kuganv@linaro.org> > > Maxim Kuvyrkov <maxim.kuvyrkov@linaro.org> > > > > PR target/65139 > > * config/aarch64/aarch64.md (<optab><mode>3): Expand lshr with > > gen_aarch64_lshr_sisd_or_int_<mode>3. > > (*aarch64_lshr_sisd_or_int_<mode>3): Rename to > > aarch64_lshr_sisd_or_int_<mode>3 and use clobber with > > match_scratch instead of early clobber. > > > > + if (strcmp ("<optab>", "lshr") == 0) > + { > > > This can't be the best way to match the operation type. Yes, I know that > the comparison is a compile time invariant, but there must be an attribute > of optab (or one can be created for it) that would make the test trivial and > not rely on the compiler optimizing the strcmp away. Perhaps just if (<CODE> == LSHIFTRT) { ? Jakub
On 15/04/15 21:59, Maxim Kuvyrkov wrote: >> On Apr 15, 2015, at 12:41 AM, Kugan <kugan.vivekanandarajah@linaro.org> wrote: >> >> This patch uses clobber with match_scratch instead of earlyclobber for >> aarch64_lshr_sisd_or_int_<mode>3 so that RA can have more freedom in >> selecting suitable register, as discussed in >> http://thread.gmane.org/gmane.comp.gcc.patches/336162 and reported in >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65139. >> >> This is based on Maxim's patch. I have bootstrapped and regression >> tested on aarch64-linux-gnu with no new regressions. Is this OK for trunk? >> >> Thanks, >> Kugan >> >> gcc/ChangeLog: >> >> 2015-04-15 Kugan Vivekanandarajah <kuganv@linaro.org> >> Maxim Kuvyrkov <maxim.kuvyrkov@linaro.org> >> >> PR target/65139 >> * config/aarch64/aarch64.md (<optab><mode>3): Expand lshr with >> gen_aarch64_lshr_sisd_or_int_<mode>3. >> (*aarch64_lshr_sisd_or_int_<mode>3): Rename to >> aarch64_lshr_sisd_or_int_<mode>3 and use clobber with >> match_scratch instead of early clobber. >> <p.txt> > > For the benefit of other reviewers -- I have reviewed this patch internally and it looks OK. > > Kugan, did you audit other patterns in aarch64.md to see if any other early-clobbers can be optimized in this way? Not yet. I will after this patch is gone through the review. Thanks, Kugan
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md index 534a862..07fa035 100644 --- a/gcc/config/aarch64/aarch64.md +++ b/gcc/config/aarch64/aarch64.md @@ -3277,6 +3277,12 @@ DONE; } } + + if (strcmp ("<optab>", "lshr") == 0) + { + emit_insn (gen_aarch64_lshr_sisd_or_int_<mode>3 (operands[0], operands[1], operands[2])); + DONE; + } } ) @@ -3361,11 +3367,12 @@ ) ;; Logical right shift using SISD or Integer instruction -(define_insn "*aarch64_lshr_sisd_or_int_<mode>3" - [(set (match_operand:GPI 0 "register_operand" "=w,&w,r") +(define_insn "aarch64_lshr_sisd_or_int_<mode>3" + [(set (match_operand:GPI 0 "register_operand" "=w,w,r") (lshiftrt:GPI (match_operand:GPI 1 "register_operand" "w,w,r") - (match_operand:QI 2 "aarch64_reg_or_shift_imm_<mode>" "Us<cmode>,w,rUs<cmode>")))] + (match_operand:QI 2 "aarch64_reg_or_shift_imm_<mode>" "Us<cmode>,w,rUs<cmode>"))) + (clobber (match_scratch:QI 3 "=X,w,X"))] "" "@ ushr\t%<rtn>0<vas>, %<rtn>1<vas>, %2 @@ -3379,30 +3386,28 @@ [(set (match_operand:DI 0 "aarch64_simd_register") (lshiftrt:DI (match_operand:DI 1 "aarch64_simd_register") - (match_operand:QI 2 "aarch64_simd_register")))] + (match_operand:QI 2 "aarch64_simd_register"))) + (clobber (match_scratch:QI 3))] "TARGET_SIMD && reload_completed" [(set (match_dup 3) (unspec:QI [(match_dup 2)] UNSPEC_SISD_NEG)) (set (match_dup 0) (unspec:DI [(match_dup 1) (match_dup 3)] UNSPEC_SISD_USHL))] - { - operands[3] = gen_lowpart (QImode, operands[0]); - } + "" ) (define_split [(set (match_operand:SI 0 "aarch64_simd_register") (lshiftrt:SI (match_operand:SI 1 "aarch64_simd_register") - (match_operand:QI 2 "aarch64_simd_register")))] + (match_operand:QI 2 "aarch64_simd_register"))) + (clobber (match_scratch:QI 3))] "TARGET_SIMD && reload_completed" [(set (match_dup 3) (unspec:QI [(match_dup 2)] UNSPEC_SISD_NEG)) (set (match_dup 0) (unspec:SI [(match_dup 1) (match_dup 3)] UNSPEC_USHL_2S))] - { - operands[3] = gen_lowpart (QImode, operands[0]); - } + "" ) ;; Arithmetic right shift using SISD or Integer instruction