| Message ID | 51A8E7DA.7080803@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Fri, May 31, 2013 at 02:11:38PM -0400, Vladimir Makarov wrote: > The patch was successfully bootstrapped and tested on s390x. > > Any comments? > Is it ok to commit the patch into trunk? > > Thanks. > > > 2013-05-31 Vladimir Makarov <vmakarov@redhat.com> > > * config/s390/s390.opt (mlra): New option. > * config/s390/s390.c (s390_decompose_address): Check displacement > for all registers for LRA. > (s390_secondary_reload): Don't used secondary reloads for LRA. > (s390_lra_p): New function. > (TARGET_LRA_P): Define. > * config/s390/s390.md (*movmem_short, *clrmem_short): Change value > of attribute cpu_facility to zarch for the last alternative. > (*cmpmem_short): Ditto. In the last chunk z900 can be used instead of zarch. The splitters are guarded with TARGET_CPU_ZARCH. This requires a zarch cpu but it does not need to run in zarch mode. z900 is the first zarch cpu. Ok with that change. Many thanks for doing the migration. We'll try to do a performance regression run soon. Bye, -Andreas-
On Fri, May 31, 2013 at 02:11:38PM -0400, Vladimir Makarov wrote: > The following patch switches LRA on for s390. The patch introduces > a new option -mlra to use LRA instead of reload. I did not document > the option as I'd like to delete it for gcc4.9. By default, LRA will > be used for s390 instead of reload for better testing LRA for s390. > > Changes in s390.md are because of define_splits for the last > alternative *movmem_short, *clrmem_short, *cmpmem_short have guard > TARGET_CPU_ZARCH. > > The patch was successfully bootstrapped and tested on s390x. > > Any comments? > Is it ok to commit the patch into trunk? > > Thanks. > > > 2013-05-31 Vladimir Makarov <vmakarov@redhat.com> > > * config/s390/s390.opt (mlra): New option. > * config/s390/s390.c (s390_decompose_address): Check displacement > for all registers for LRA. > (s390_secondary_reload): Don't used secondary reloads for LRA. > (s390_lra_p): New function. > (TARGET_LRA_P): Define. > * config/s390/s390.md (*movmem_short, *clrmem_short): Change value > of attribute cpu_facility to zarch for the last alternative. > (*cmpmem_short): Ditto. I've applied the attached patch. This helps me getting a little further when bootstrapping with lra and --with-arch=zEC12. 2013-06-07 Andreas Krebbel <Andreas.Krebbel@de.ibm.com> * config/s390/s390.md (cpu_facility): Add cpu_zarch. ("*movmem_short", "*clrmem_short", "*cmpmem_short): Use cpu_zarch for last alternative in the cpu_facility attribute. --- gcc/config/s390/s390.md | 13 ++++!!!!!!!!! 1 file changed, 4 insertions(+), 9 modifications(!) Index: gcc/config/s390/s390.md =================================================================== *** gcc/config/s390/s390.md.orig --- gcc/config/s390/s390.md *************** *** 277,283 **** (define_attr "cpu" "g5,g6,z900,z990,z9_109,z9_ec,z10,z196,zEC12" (const (symbol_ref "s390_tune_attr"))) ! (define_attr "cpu_facility" "standard,ieee,zarch,longdisp,extimm,dfp,z10,z196,zEC12" (const_string "standard")) (define_attr "enabled" "" --- 277,284 ---- (define_attr "cpu" "g5,g6,z900,z990,z9_109,z9_ec,z10,z196,zEC12" (const (symbol_ref "s390_tune_attr"))) ! (define_attr "cpu_facility" ! "standard,ieee,zarch,cpu_zarch,longdisp,extimm,dfp,z10,z196,zEC12" (const_string "standard")) (define_attr "enabled" "" *************** *** 304,309 **** --- 305,314 ---- (match_test "TARGET_DFP")) (const_int 1) + (and (eq_attr "cpu_facility" "cpu_zarch") + (match_test "TARGET_CPU_ZARCH")) + (const_int 1) + (and (eq_attr "cpu_facility" "z10") (match_test "TARGET_Z10")) (const_int 1) *************** *** 2690,2696 **** "(GET_MODE (operands[2]) == Pmode || GET_MODE (operands[2]) == VOIDmode)" "#" [(set_attr "type" "cs") ! (set_attr "cpu_facility" "*,*,z10,zarch")]) (define_split [(set (match_operand:BLK 0 "memory_operand" "") --- 2695,2701 ---- "(GET_MODE (operands[2]) == Pmode || GET_MODE (operands[2]) == VOIDmode)" "#" [(set_attr "type" "cs") ! (set_attr "cpu_facility" "*,*,z10,cpu_zarch")]) (define_split [(set (match_operand:BLK 0 "memory_operand" "") *************** *** 2899,2905 **** "(GET_MODE (operands[1]) == Pmode || GET_MODE (operands[1]) == VOIDmode)" "#" [(set_attr "type" "cs") ! (set_attr "cpu_facility" "*,*,z10,zarch")]) (define_split [(set (match_operand:BLK 0 "memory_operand" "") --- 2904,2910 ---- "(GET_MODE (operands[1]) == Pmode || GET_MODE (operands[1]) == VOIDmode)" "#" [(set_attr "type" "cs") ! (set_attr "cpu_facility" "*,*,z10,cpu_zarch")]) (define_split [(set (match_operand:BLK 0 "memory_operand" "") *************** *** 3075,3081 **** "(GET_MODE (operands[2]) == Pmode || GET_MODE (operands[2]) == VOIDmode)" "#" [(set_attr "type" "cs") ! (set_attr "cpu_facility" "*,*,z10,zarch")]) (define_split [(set (reg:CCU CC_REGNUM) --- 3080,3086 ---- "(GET_MODE (operands[2]) == Pmode || GET_MODE (operands[2]) == VOIDmode)" "#" [(set_attr "type" "cs") ! (set_attr "cpu_facility" "*,*,z10,cpu_zarch")]) (define_split [(set (reg:CCU CC_REGNUM)
On 13-06-07 10:57 AM, Andreas Krebbel wrote: > On Fri, May 31, 2013 at 02:11:38PM -0400, Vladimir Makarov wrote: >> The following patch switches LRA on for s390. The patch introduces >> a new option -mlra to use LRA instead of reload. I did not document >> the option as I'd like to delete it for gcc4.9. By default, LRA will >> be used for s390 instead of reload for better testing LRA for s390. >> >> Changes in s390.md are because of define_splits for the last >> alternative *movmem_short, *clrmem_short, *cmpmem_short have guard >> TARGET_CPU_ZARCH. >> >> The patch was successfully bootstrapped and tested on s390x. >> >> Any comments? >> Is it ok to commit the patch into trunk? >> >> Thanks. >> >> >> 2013-05-31 Vladimir Makarov <vmakarov@redhat.com> >> >> * config/s390/s390.opt (mlra): New option. >> * config/s390/s390.c (s390_decompose_address): Check displacement >> for all registers for LRA. >> (s390_secondary_reload): Don't used secondary reloads for LRA. >> (s390_lra_p): New function. >> (TARGET_LRA_P): Define. >> * config/s390/s390.md (*movmem_short, *clrmem_short): Change value >> of attribute cpu_facility to zarch for the last alternative. >> (*cmpmem_short): Ditto. > I've applied the attached patch. This helps me getting a little > further when bootstrapping with lra and --with-arch=zEC12. > > 2013-06-07 Andreas Krebbel <Andreas.Krebbel@de.ibm.com> > > * config/s390/s390.md (cpu_facility): Add cpu_zarch. > ("*movmem_short", "*clrmem_short", "*cmpmem_short): Use cpu_zarch > for last alternative in the cpu_facility attribute. > > --- > gcc/config/s390/s390.md | 13 ++++!!!!!!!!! > 1 file changed, 4 insertions(+), 9 modifications(!) > > Index: gcc/config/s390/s390.md > =================================================================== > *** gcc/config/s390/s390.md.orig > --- gcc/config/s390/s390.md > *************** > *** 277,283 **** > (define_attr "cpu" "g5,g6,z900,z990,z9_109,z9_ec,z10,z196,zEC12" > (const (symbol_ref "s390_tune_attr"))) > > ! (define_attr "cpu_facility" "standard,ieee,zarch,longdisp,extimm,dfp,z10,z196,zEC12" > (const_string "standard")) > > (define_attr "enabled" "" > --- 277,284 ---- > (define_attr "cpu" "g5,g6,z900,z990,z9_109,z9_ec,z10,z196,zEC12" > (const (symbol_ref "s390_tune_attr"))) > > ! (define_attr "cpu_facility" > ! "standard,ieee,zarch,cpu_zarch,longdisp,extimm,dfp,z10,z196,zEC12" > (const_string "standard")) > > (define_attr "enabled" "" > *************** > *** 304,309 **** > --- 305,314 ---- > (match_test "TARGET_DFP")) > (const_int 1) > > + (and (eq_attr "cpu_facility" "cpu_zarch") > + (match_test "TARGET_CPU_ZARCH")) > + (const_int 1) > + > (and (eq_attr "cpu_facility" "z10") > (match_test "TARGET_Z10")) > (const_int 1) > *************** > *** 2690,2696 **** > "(GET_MODE (operands[2]) == Pmode || GET_MODE (operands[2]) == VOIDmode)" > "#" > [(set_attr "type" "cs") > ! (set_attr "cpu_facility" "*,*,z10,zarch")]) > > (define_split > [(set (match_operand:BLK 0 "memory_operand" "") > --- 2695,2701 ---- > "(GET_MODE (operands[2]) == Pmode || GET_MODE (operands[2]) == VOIDmode)" > "#" > [(set_attr "type" "cs") > ! (set_attr "cpu_facility" "*,*,z10,cpu_zarch")]) > > (define_split > [(set (match_operand:BLK 0 "memory_operand" "") > *************** > *** 2899,2905 **** > "(GET_MODE (operands[1]) == Pmode || GET_MODE (operands[1]) == VOIDmode)" > "#" > [(set_attr "type" "cs") > ! (set_attr "cpu_facility" "*,*,z10,zarch")]) > > (define_split > [(set (match_operand:BLK 0 "memory_operand" "") > --- 2904,2910 ---- > "(GET_MODE (operands[1]) == Pmode || GET_MODE (operands[1]) == VOIDmode)" > "#" > [(set_attr "type" "cs") > ! (set_attr "cpu_facility" "*,*,z10,cpu_zarch")]) > > (define_split > [(set (match_operand:BLK 0 "memory_operand" "") > *************** > *** 3075,3081 **** > "(GET_MODE (operands[2]) == Pmode || GET_MODE (operands[2]) == VOIDmode)" > "#" > [(set_attr "type" "cs") > ! (set_attr "cpu_facility" "*,*,z10,zarch")]) > > (define_split > [(set (reg:CCU CC_REGNUM) > --- 3080,3086 ---- > "(GET_MODE (operands[2]) == Pmode || GET_MODE (operands[2]) == VOIDmode)" > "#" > [(set_attr "type" "cs") > ! (set_attr "cpu_facility" "*,*,z10,cpu_zarch")]) > > (define_split > [(set (reg:CCU CC_REGNUM) > Thanks, Andreas. I am not a specialist in 390 architecture. You are the expert. I'll check the bootstrap with this option too on a machine available for me. I've checked the bootstrap without any -with-arch option before submitting the patch for approval. If I find some problems, I'll work on them too. I'll work on PR57599 you just reported too.
On 13-06-07 11:12 AM, Vladimir Makarov wrote: > On 13-06-07 10:57 AM, Andreas Krebbel wrote: >> I've applied the attached patch. This helps me getting a little >> further when bootstrapping with lra and --with-arch=zEC12. >> >> 2013-06-07 Andreas Krebbel <Andreas.Krebbel@de.ibm.com> >> >> * config/s390/s390.md (cpu_facility): Add cpu_zarch. >> ("*movmem_short", "*clrmem_short", "*cmpmem_short): Use cpu_zarch >> for last alternative in the cpu_facility attribute. >> >> --- >> gcc/config/s390/s390.md | 13 ++++!!!!!!!!! >> 1 file changed, 4 insertions(+), 9 modifications(!) >> >> > Thanks, Andreas. I am not a specialist in 390 architecture. You are > the expert. I'll check the bootstrap with this option too on a > machine available for me. I've checked the bootstrap without any > -with-arch option before submitting the patch for approval. If I find > some problems, I'll work on them too. I'll work on PR57599 you just > reported too. > I've fixed PR57599. Unfortunately, I can not bootstrap with --with-arch=zEC12 as a machine available for me does not support this subtarget.
On 08/06/13 20:41, Vladimir Makarov wrote: > On 13-06-07 11:12 AM, Vladimir Makarov wrote: >> On 13-06-07 10:57 AM, Andreas Krebbel wrote: >>> I've applied the attached patch. This helps me getting a little >>> further when bootstrapping with lra and --with-arch=zEC12. >>> >>> 2013-06-07 Andreas Krebbel <Andreas.Krebbel@de.ibm.com> >>> >>> * config/s390/s390.md (cpu_facility): Add cpu_zarch. >>> ("*movmem_short", "*clrmem_short", "*cmpmem_short): Use cpu_zarch >>> for last alternative in the cpu_facility attribute. >>> >>> --- >>> gcc/config/s390/s390.md | 13 ++++!!!!!!!!! >>> 1 file changed, 4 insertions(+), 9 modifications(!) >>> >>> >> Thanks, Andreas. I am not a specialist in 390 architecture. You are >> the expert. I'll check the bootstrap with this option too on a >> machine available for me. I've checked the bootstrap without any >> -with-arch option before submitting the patch for approval. If I find >> some problems, I'll work on them too. I'll work on PR57599 you just >> reported too. >> > I've fixed PR57599. Unfortunately, I can not bootstrap with > --with-arch=zEC12 as a machine available for me does not support this > subtarget. Thanks. I would be happy to test the patch for you. Bye, -Andreas-
Index: config/s390/s390.c =================================================================== --- config/s390/s390.c (revision 199453) +++ config/s390/s390.c (working copy) @@ -2017,14 +2017,18 @@ s390_decompose_address (rtx addr, struct Thus we don't check the displacement for validity here. If after elimination the displacement turns out to be invalid after all, this is fixed up by reload in any case. */ - if (base != arg_pointer_rtx - && indx != arg_pointer_rtx - && base != return_address_pointer_rtx - && indx != return_address_pointer_rtx - && base != frame_pointer_rtx - && indx != frame_pointer_rtx - && base != virtual_stack_vars_rtx - && indx != virtual_stack_vars_rtx) + /* LRA maintains always displacements up to date and we need to + know the displacement is right during all LRA not only at the + final elimination. */ + if (lra_in_progress + || (base != arg_pointer_rtx + && indx != arg_pointer_rtx + && base != return_address_pointer_rtx + && indx != return_address_pointer_rtx + && base != frame_pointer_rtx + && indx != frame_pointer_rtx + && base != virtual_stack_vars_rtx + && indx != virtual_stack_vars_rtx)) if (!DISP_IN_RANGE (offset)) return false; } @@ -3189,7 +3193,9 @@ s390_secondary_reload (bool in_p, rtx x, /* We need a scratch register when loading a PLUS expression which is not a legitimate operand of the LOAD ADDRESS instruction. */ - if (in_p && s390_plus_operand (x, mode)) + /* LRA can deal with transformation of plus op very well -- so we + don't need to prompt LRA in this case. */ + if (! lra_in_progress && in_p && s390_plus_operand (x, mode)) sri->icode = (TARGET_64BIT ? CODE_FOR_reloaddi_plus : CODE_FOR_reloadsi_plus); @@ -7868,6 +7874,13 @@ s390_class_max_nregs (enum reg_class rcl return (GET_MODE_SIZE (mode) + UNITS_PER_WORD - 1) / UNITS_PER_WORD; } +/* Return true if we use LRA instead of reload pass. */ +static bool +s390_lra_p (void) +{ + return s390_lra_flag; +} + /* Return true if register FROM can be eliminated via register TO. */ static bool @@ -11105,6 +11118,9 @@ s390_loop_unroll_adjust (unsigned nunrol #undef TARGET_LEGITIMATE_CONSTANT_P #define TARGET_LEGITIMATE_CONSTANT_P s390_legitimate_constant_p +#undef TARGET_LRA_P +#define TARGET_LRA_P s390_lra_p + #undef TARGET_CAN_ELIMINATE #define TARGET_CAN_ELIMINATE s390_can_eliminate Index: config/s390/s390.md =================================================================== --- config/s390/s390.md (revision 199453) +++ config/s390/s390.md (working copy) @@ -2690,7 +2690,7 @@ "(GET_MODE (operands[2]) == Pmode || GET_MODE (operands[2]) == VOIDmode)" "#" [(set_attr "type" "cs") - (set_attr "cpu_facility" "*,*,z10,*")]) + (set_attr "cpu_facility" "*,*,z10,zarch")]) (define_split [(set (match_operand:BLK 0 "memory_operand" "") @@ -2899,7 +2899,7 @@ "(GET_MODE (operands[1]) == Pmode || GET_MODE (operands[1]) == VOIDmode)" "#" [(set_attr "type" "cs") - (set_attr "cpu_facility" "*,*,z10,*")]) + (set_attr "cpu_facility" "*,*,z10,zarch")]) (define_split [(set (match_operand:BLK 0 "memory_operand" "") @@ -3075,7 +3075,7 @@ "(GET_MODE (operands[2]) == Pmode || GET_MODE (operands[2]) == VOIDmode)" "#" [(set_attr "type" "cs") - (set_attr "cpu_facility" "*,*,z10,*")]) + (set_attr "cpu_facility" "*,*,z10,zarch")]) (define_split [(set (reg:CCU CC_REGNUM) Index: config/s390/s390.opt =================================================================== --- config/s390/s390.opt (revision 199453) +++ config/s390/s390.opt (working copy) @@ -149,3 +149,7 @@ Target Report Joined RejectNegative UInt Set the branch costs for conditional branch instructions. Reasonable values are small, non-negative integers. The default branch cost is 1. + +mlra +Target Report Var(s390_lra_flag) Init(1) Save +Use LRA instead of reload
The following patch switches LRA on for s390. The patch introduces a new option -mlra to use LRA instead of reload. I did not document the option as I'd like to delete it for gcc4.9. By default, LRA will be used for s390 instead of reload for better testing LRA for s390. Changes in s390.md are because of define_splits for the last alternative *movmem_short, *clrmem_short, *cmpmem_short have guard TARGET_CPU_ZARCH. The patch was successfully bootstrapped and tested on s390x. Any comments? Is it ok to commit the patch into trunk? Thanks. 2013-05-31 Vladimir Makarov <vmakarov@redhat.com> * config/s390/s390.opt (mlra): New option. * config/s390/s390.c (s390_decompose_address): Check displacement for all registers for LRA. (s390_secondary_reload): Don't used secondary reloads for LRA. (s390_lra_p): New function. (TARGET_LRA_P): Define. * config/s390/s390.md (*movmem_short, *clrmem_short): Change value of attribute cpu_facility to zarch for the last alternative. (*cmpmem_short): Ditto.