| Message ID | 12e6eeea-ba32-bdcf-1e42-6480b0bbad32@mentor.com |
|---|---|
| State | New |
| Headers | show |
| Series | [PR83327] Fix liveness analysis in lra for spilled-into hard regs | expand |
On 12/15/2017 06:25 AM, Tom de Vries wrote: > > Proposed Solution: > > The patch addresses the problem, by: > - marking the hard regs that have been used in lra_spill in > hard_regs_spilled_into > - using hard_regs_spilled_into in lra_create_live_ranges to > make sure those registers are marked in the conflict_hard_regs > of pseudos that overlap with the spill register usage > > [ I've also tried an approach where I didn't use > hard_regs_spilled_into, but tried to propagate all hard regs. I > figured out that I needed to mask out eliminable_regset. Also I > needed to masked out lra_no_alloc_regs, but that could be due to > gcn-specific problems (pointers take 2 hard regs), I'm not yet sure. > Anyway, in the submitted patch I tried to avoid these problems and > went for the more minimal approach. ] > Tom, thank you for the detail explanation of the problem and solutions you considered. It helped me a lot. Your simple solution is adequate as the most transformations and allocation are done on the 1st LRA subpasses iteration. > In order to get the patch accepted for trunk, I think we need: > - bootstrap and reg-test on x86_64 > - build and reg-test on mips (the only primary platform that has the > spill_class hook enabled) > > Any comments? The patch looks ok to me. You can commit it after successful testing on x86-64 and mips but I am sure there will be no problems with x86-64 as it does not use spill_class currently (actually your patch might help to switch it on again for x86-64. spill_class was quite useful for x86-64 performance on Intel processors).
On 12/18/2017 05:57 PM, Vladimir Makarov wrote: > > > On 12/15/2017 06:25 AM, Tom de Vries wrote: >> >> Proposed Solution: >> >> The patch addresses the problem, by: >> - marking the hard regs that have been used in lra_spill in >> hard_regs_spilled_into >> - using hard_regs_spilled_into in lra_create_live_ranges to >> make sure those registers are marked in the conflict_hard_regs >> of pseudos that overlap with the spill register usage >> >> [ I've also tried an approach where I didn't use >> hard_regs_spilled_into, but tried to propagate all hard regs. I >> figured out that I needed to mask out eliminable_regset. Also I >> needed to masked out lra_no_alloc_regs, but that could be due to >> gcn-specific problems (pointers take 2 hard regs), I'm not yet sure. >> Anyway, in the submitted patch I tried to avoid these problems and >> went for the more minimal approach. ] >> > Tom, thank you for the detail explanation of the problem and solutions > you considered. It helped me a lot. Your simple solution is adequate > as the most transformations and allocation are done on the 1st LRA > subpasses iteration. >> In order to get the patch accepted for trunk, I think we need: >> - bootstrap and reg-test on x86_64 >> - build and reg-test on mips (the only primary platform that has the >> spill_class hook enabled) >> >> Any comments? > > The patch looks ok to me. You can commit it after successful testing on > x86-64 and mips but I am sure there will be no problems with x86-64 as > it does not use spill_class currently (actually your patch might help to > switch it on again for x86-64. spill_class was quite useful for x86-64 > performance on Intel processors). > Hi Matthew, there's an lra optimization that is currently enabled for MIPS, and not for any other primary or secondary target. This (already approved) patch fixes a bug in that optimization, and needs to be tested on MIPS. Unfortunately, the optimization is only enabled for MIPS16, and we don't have a current setup to test this. Could you help us out here and test this patch for MIPS16 on trunk? Thanks, - Tom
On 01/08/2018 05:32 PM, Tom de Vries wrote: > On 12/18/2017 05:57 PM, Vladimir Makarov wrote: >> >> >> On 12/15/2017 06:25 AM, Tom de Vries wrote: >>> >>> Proposed Solution: >>> >>> The patch addresses the problem, by: >>> - marking the hard regs that have been used in lra_spill in >>> hard_regs_spilled_into >>> - using hard_regs_spilled_into in lra_create_live_ranges to >>> make sure those registers are marked in the conflict_hard_regs >>> of pseudos that overlap with the spill register usage >>> >>> [ I've also tried an approach where I didn't use >>> hard_regs_spilled_into, but tried to propagate all hard regs. I >>> figured out that I needed to mask out eliminable_regset. Also I >>> needed to masked out lra_no_alloc_regs, but that could be due to >>> gcn-specific problems (pointers take 2 hard regs), I'm not yet sure. >>> Anyway, in the submitted patch I tried to avoid these problems and >>> went for the more minimal approach. ] >>> >> Tom, thank you for the detail explanation of the problem and solutions >> you considered. It helped me a lot. Your simple solution is adequate >> as the most transformations and allocation are done on the 1st LRA >> subpasses iteration. >>> In order to get the patch accepted for trunk, I think we need: >>> - bootstrap and reg-test on x86_64 >>> - build and reg-test on mips (the only primary platform that has the >>> spill_class hook enabled) >>> >>> Any comments? >> >> The patch looks ok to me. You can commit it after successful testing >> on x86-64 and mips but I am sure there will be no problems with x86-64 >> as it does not use spill_class currently (actually your patch might >> help to switch it on again for x86-64. spill_class was quite useful >> for x86-64 performance on Intel processors). >> > > Hi Matthew, > > there's an lra optimization that is currently enabled for MIPS, and not > for any other primary or secondary target. > > This (already approved) patch fixes a bug in that optimization, and > needs to be tested on MIPS. > > Unfortunately, the optimization is only enabled for MIPS16, and we don't > have a current setup to test this. > > Could you help us out here and test this patch for MIPS16 on trunk? Hi Matthew, is this something you can help us out with? Thanks, - Tom
Tom de Vries <Tom_deVries@mentor.com> writes: > On 01/08/2018 05:32 PM, Tom de Vries wrote: > > On 12/18/2017 05:57 PM, Vladimir Makarov wrote: > >> > >> > >> On 12/15/2017 06:25 AM, Tom de Vries wrote: > >>> > >>> Proposed Solution: > >>> > >>> The patch addresses the problem, by: > >>> - marking the hard regs that have been used in lra_spill in > >>> hard_regs_spilled_into > >>> - using hard_regs_spilled_into in lra_create_live_ranges to > >>> make sure those registers are marked in the conflict_hard_regs > >>> of pseudos that overlap with the spill register usage > >>> > >>> [ I've also tried an approach where I didn't use > >>> hard_regs_spilled_into, but tried to propagate all hard regs. I > >>> figured out that I needed to mask out eliminable_regset. Also I > >>> needed to masked out lra_no_alloc_regs, but that could be due to > >>> gcn-specific problems (pointers take 2 hard regs), I'm not yet sure. > >>> Anyway, in the submitted patch I tried to avoid these problems and > >>> went for the more minimal approach. ] > >>> > >> Tom, thank you for the detail explanation of the problem and > >> solutions you considered. It helped me a lot. Your simple solution > >> is adequate as the most transformations and allocation are done on > >> the 1st LRA subpasses iteration. > >>> In order to get the patch accepted for trunk, I think we need: > >>> - bootstrap and reg-test on x86_64 > >>> - build and reg-test on mips (the only primary platform that has the > >>> spill_class hook enabled) > >>> > >>> Any comments? > >> > >> The patch looks ok to me. You can commit it after successful testing > >> on x86-64 and mips but I am sure there will be no problems with > >> x86-64 as it does not use spill_class currently (actually your patch > >> might help to switch it on again for x86-64. spill_class was quite > >> useful for x86-64 performance on Intel processors). > >> > > > > Hi Matthew, > > > > there's an lra optimization that is currently enabled for MIPS, and > > not for any other primary or secondary target. > > > > This (already approved) patch fixes a bug in that optimization, and > > needs to be tested on MIPS. > > > > Unfortunately, the optimization is only enabled for MIPS16, and we > > don't have a current setup to test this. > > > > Could you help us out here and test this patch for MIPS16 on trunk? > > Hi Matthew, > > is this something you can help us out with? Hi Tom, I've just commented on the bug report that I've set of some builds to try and give some assurance. It is far from comprehensive but it is as good as the normal testing I do for MIPS16. Thanks, Matthew
On 02/26/2018 12:00 PM, Matthew Fortune wrote: > Tom de Vries <Tom_deVries@mentor.com> writes: >> On 01/08/2018 05:32 PM, Tom de Vries wrote: >>> On 12/18/2017 05:57 PM, Vladimir Makarov wrote: >>>> >>>> >>>> On 12/15/2017 06:25 AM, Tom de Vries wrote: >>>>> >>>>> Proposed Solution: >>>>> >>>>> The patch addresses the problem, by: >>>>> - marking the hard regs that have been used in lra_spill in >>>>> hard_regs_spilled_into >>>>> - using hard_regs_spilled_into in lra_create_live_ranges to >>>>> make sure those registers are marked in the conflict_hard_regs >>>>> of pseudos that overlap with the spill register usage >>>>> >>>>> [ I've also tried an approach where I didn't use >>>>> hard_regs_spilled_into, but tried to propagate all hard regs. I >>>>> figured out that I needed to mask out eliminable_regset. Also I >>>>> needed to masked out lra_no_alloc_regs, but that could be due to >>>>> gcn-specific problems (pointers take 2 hard regs), I'm not yet sure. >>>>> Anyway, in the submitted patch I tried to avoid these problems and >>>>> went for the more minimal approach. ] >>>>> >>>> Tom, thank you for the detail explanation of the problem and >>>> solutions you considered. It helped me a lot. Your simple solution >>>> is adequate as the most transformations and allocation are done on >>>> the 1st LRA subpasses iteration. >>>>> In order to get the patch accepted for trunk, I think we need: >>>>> - bootstrap and reg-test on x86_64 >>>>> - build and reg-test on mips (the only primary platform that has the >>>>> spill_class hook enabled) >>>>> >>>>> Any comments? >>>> >>>> The patch looks ok to me. You can commit it after successful testing >>>> on x86-64 and mips but I am sure there will be no problems with >>>> x86-64 as it does not use spill_class currently (actually your patch >>>> might help to switch it on again for x86-64. spill_class was quite >>>> useful for x86-64 performance on Intel processors). >>>> >>> >>> Hi Matthew, >>> >>> there's an lra optimization that is currently enabled for MIPS, and >>> not for any other primary or secondary target. >>> >>> This (already approved) patch fixes a bug in that optimization, and >>> needs to be tested on MIPS. >>> >>> Unfortunately, the optimization is only enabled for MIPS16, and we >>> don't have a current setup to test this. >>> >>> Could you help us out here and test this patch for MIPS16 on trunk? >> >> Hi Matthew, >> >> is this something you can help us out with? > > Hi Tom, > > I've just commented on the bug report that I've set of some builds to try > and give some assurance. It is far from comprehensive but it is as good as > the normal testing I do for MIPS16. > Hi Matthew, Awesome, thanks for the help. - Tom
Tom de Vries <Tom_deVries@mentor.com> writes: > On 02/26/2018 12:00 PM, Matthew Fortune wrote: > > Tom de Vries <Tom_deVries@mentor.com> writes: > >> On 01/08/2018 05:32 PM, Tom de Vries wrote: > >>> On 12/18/2017 05:57 PM, Vladimir Makarov wrote: > >>>> > >>>> > >>>> On 12/15/2017 06:25 AM, Tom de Vries wrote: > >>>>> > >>>>> Proposed Solution: > >>>>> > >>>>> The patch addresses the problem, by: > >>>>> - marking the hard regs that have been used in lra_spill in > >>>>> hard_regs_spilled_into > >>>>> - using hard_regs_spilled_into in lra_create_live_ranges to > >>>>> make sure those registers are marked in the conflict_hard_regs > >>>>> of pseudos that overlap with the spill register usage > >>>>> > >>>>> [ I've also tried an approach where I didn't use > >>>>> hard_regs_spilled_into, but tried to propagate all hard regs. I > >>>>> figured out that I needed to mask out eliminable_regset. Also I > >>>>> needed to masked out lra_no_alloc_regs, but that could be due to > >>>>> gcn-specific problems (pointers take 2 hard regs), I'm not yet > sure. > >>>>> Anyway, in the submitted patch I tried to avoid these problems and > >>>>> went for the more minimal approach. ] > >>>>> > >>>> Tom, thank you for the detail explanation of the problem and > >>>> solutions you considered. It helped me a lot. Your simple > >>>> solution is adequate as the most transformations and allocation are > >>>> done on the 1st LRA subpasses iteration. > >>>>> In order to get the patch accepted for trunk, I think we need: > >>>>> - bootstrap and reg-test on x86_64 > >>>>> - build and reg-test on mips (the only primary platform that has > >>>>> the > >>>>> spill_class hook enabled) > >>>>> > >>>>> Any comments? > >>>> > >>>> The patch looks ok to me. You can commit it after successful > >>>> testing on x86-64 and mips but I am sure there will be no problems > >>>> with > >>>> x86-64 as it does not use spill_class currently (actually your > >>>> patch might help to switch it on again for x86-64. spill_class was > >>>> quite useful for x86-64 performance on Intel processors). > >>>> > >>> > >>> Hi Matthew, > >>> > >>> there's an lra optimization that is currently enabled for MIPS, and > >>> not for any other primary or secondary target. > >>> > >>> This (already approved) patch fixes a bug in that optimization, and > >>> needs to be tested on MIPS. > >>> > >>> Unfortunately, the optimization is only enabled for MIPS16, and we > >>> don't have a current setup to test this. > >>> > >>> Could you help us out here and test this patch for MIPS16 on trunk? > >> > >> Hi Matthew, > >> > >> is this something you can help us out with? > > > > Hi Tom, > > > > I've just commented on the bug report that I've set of some builds to > > try and give some assurance. It is far from comprehensive but it is as > > good as the normal testing I do for MIPS16. > > > > Hi Matthew, > > Awesome, thanks for the help. I'm afraid my lack of attention has shown that there appears to be a bug preventing libstdc++ building with MIPS16 that was introduced somewhere between Oct 9th and Oct 10th. 47 commits left to bisect. Matthew
Tom de Vries <Tom_deVries@mentor.com> writes: > On 02/26/2018 12:00 PM, Matthew Fortune wrote: > > Tom de Vries <Tom_deVries@mentor.com> writes: > >> On 01/08/2018 05:32 PM, Tom de Vries wrote: > >>> On 12/18/2017 05:57 PM, Vladimir Makarov wrote: > >>>> > >>>> > >>>> On 12/15/2017 06:25 AM, Tom de Vries wrote: > >>>>> > >>>>> Proposed Solution: > >>>>> > >>>>> The patch addresses the problem, by: > >>>>> - marking the hard regs that have been used in lra_spill in > >>>>> hard_regs_spilled_into > >>>>> - using hard_regs_spilled_into in lra_create_live_ranges to > >>>>> make sure those registers are marked in the conflict_hard_regs > >>>>> of pseudos that overlap with the spill register usage > >>>>> > >>>>> [ I've also tried an approach where I didn't use > >>>>> hard_regs_spilled_into, but tried to propagate all hard regs. I > >>>>> figured out that I needed to mask out eliminable_regset. Also I > >>>>> needed to masked out lra_no_alloc_regs, but that could be due to > >>>>> gcn-specific problems (pointers take 2 hard regs), I'm not yet > sure. > >>>>> Anyway, in the submitted patch I tried to avoid these problems > and > >>>>> went for the more minimal approach. ] > >>>>> > >>>> Tom, thank you for the detail explanation of the problem and > >>>> solutions you considered. It helped me a lot. Your simple > solution > >>>> is adequate as the most transformations and allocation are done on > >>>> the 1st LRA subpasses iteration. > >>>>> In order to get the patch accepted for trunk, I think we need: > >>>>> - bootstrap and reg-test on x86_64 > >>>>> - build and reg-test on mips (the only primary platform that has > the > >>>>> spill_class hook enabled) > >>>>> > >>>>> Any comments? > >>>> > >>>> The patch looks ok to me. You can commit it after successful > testing > >>>> on x86-64 and mips but I am sure there will be no problems with > >>>> x86-64 as it does not use spill_class currently (actually your > patch > >>>> might help to switch it on again for x86-64. spill_class was > quite > >>>> useful for x86-64 performance on Intel processors). > >>>> > >>> > >>> Hi Matthew, > >>> > >>> there's an lra optimization that is currently enabled for MIPS, and > >>> not for any other primary or secondary target. > >>> > >>> This (already approved) patch fixes a bug in that optimization, and > >>> needs to be tested on MIPS. > >>> > >>> Unfortunately, the optimization is only enabled for MIPS16, and we > >>> don't have a current setup to test this. > >>> > >>> Could you help us out here and test this patch for MIPS16 on trunk? > >> > >> Hi Matthew, > >> > >> is this something you can help us out with? > > > > Hi Tom, > > > > I've just commented on the bug report that I've set of some builds to > try > > and give some assurance. It is far from comprehensive but it is as > good as > > the normal testing I do for MIPS16. > > > > Hi Matthew, > > Awesome, thanks for the help. I have tested trunk with and without the patch and can confirm there is no change in test status for MIPS16 big endian. I ended up fixing an assert-checking issue in the MIPS backend and a bug (I think) in the C++ frontend to get to the point of testing so quite a worthwhile effort all in all. Thanks, Matthew
Fix liveness analysis in lra for spilled-into hard regs 2017-12-12 Tom de Vries <tom@codesourcery.com> PR rtl-optimization/83327 * lra-int.h (hard_regs_spilled_into): Declare. * lra.c (hard_regs_spilled_into): Define. (init_reg_info): Init hard_regs_spilled_into. * lra-spills.c (assign_spill_hard_regs): Update hard_regs_spilled_into. * lra-lives.c (make_hard_regno_born, make_hard_regno_dead) (process_bb_lives): Handle hard_regs_spilled_into. (lra_create_live_ranges_1): Before doing liveness propagation, clear regs in all_hard_regs_bitmap if set in hard_regs_spilled_into. --- gcc/lra-int.h | 2 ++ gcc/lra-lives.c | 28 ++++++++++++++++++++++++++-- gcc/lra-spills.c | 2 ++ gcc/lra.c | 3 +++ 4 files changed, 33 insertions(+), 2 deletions(-) diff --git a/gcc/lra-int.h b/gcc/lra-int.h index 5a519b0..064961a 100644 --- a/gcc/lra-int.h +++ b/gcc/lra-int.h @@ -122,6 +122,8 @@ struct lra_reg /* References to the common info about each register. */ extern struct lra_reg *lra_reg_info; +extern HARD_REG_SET hard_regs_spilled_into; + /* Static info about each insn operand (common for all insns with the same ICODE). Warning: if the structure definition is changed, the initializer for debug_operand_data in lra.c should be changed diff --git a/gcc/lra-lives.c b/gcc/lra-lives.c index 03dfec2..85da626 100644 --- a/gcc/lra-lives.c +++ b/gcc/lra-lives.c @@ -245,7 +245,7 @@ make_hard_regno_born (int regno, bool check_pic_pseudo_p ATTRIBUTE_UNUSED) || i != REGNO (pic_offset_table_rtx)) #endif SET_HARD_REG_BIT (lra_reg_info[i].conflict_hard_regs, regno); - if (fixed_regs[regno]) + if (fixed_regs[regno] || TEST_HARD_REG_BIT (hard_regs_spilled_into, regno)) bitmap_set_bit (bb_gen_pseudos, regno); } @@ -259,7 +259,7 @@ make_hard_regno_dead (int regno) return; sparseset_set_bit (start_dying, regno); CLEAR_HARD_REG_BIT (hard_regs_live, regno); - if (fixed_regs[regno]) + if (fixed_regs[regno] || TEST_HARD_REG_BIT (hard_regs_spilled_into, regno)) { bitmap_clear_bit (bb_gen_pseudos, regno); bitmap_set_bit (bb_killed_pseudos, regno); @@ -1051,6 +1051,25 @@ process_bb_lives (basic_block bb, int &curr_point, bool dead_insn_p) check_pseudos_live_through_calls (j, last_call_used_reg_set); } + for (i = 0; i < FIRST_PSEUDO_REGISTER; ++i) + { + if (!TEST_HARD_REG_BIT (hard_regs_live, i)) + continue; + + if (!TEST_HARD_REG_BIT (hard_regs_spilled_into, i)) + continue; + + if (bitmap_bit_p (df_get_live_in (bb), i)) + continue; + + live_change_p = true; + if (lra_dump_file) + fprintf (lra_dump_file, + " hard reg r%d is added to live at bb%d start\n", i, + bb->index); + bitmap_set_bit (df_get_live_in (bb), i); + } + if (need_curr_point_incr) next_program_point (curr_point, freq); @@ -1320,6 +1339,11 @@ lra_create_live_ranges_1 (bool all_p, bool dead_insn_p) } /* As we did not change CFG since LRA start we can use DF-infrastructure solver to solve live data flow problem. */ + for (int i = 0; i < FIRST_PSEUDO_REGISTER; ++i) + { + if (TEST_HARD_REG_BIT (hard_regs_spilled_into, i)) + bitmap_clear_bit (&all_hard_regs_bitmap, i); + } df_simple_dataflow (DF_BACKWARD, NULL, live_con_fun_0, live_con_fun_n, live_trans_fun, &all_blocks, diff --git a/gcc/lra-spills.c b/gcc/lra-spills.c index af11b3d..ab13b21 100644 --- a/gcc/lra-spills.c +++ b/gcc/lra-spills.c @@ -291,6 +291,8 @@ assign_spill_hard_regs (int *pseudo_regnos, int n) } if (lra_dump_file != NULL) fprintf (lra_dump_file, " Spill r%d into hr%d\n", regno, hard_regno); + add_to_hard_reg_set (&hard_regs_spilled_into, + lra_reg_info[regno].biggest_mode, hard_regno); /* Update reserved_hard_regs. */ for (r = lra_reg_info[regno].live_ranges; r != NULL; r = r->next) for (p = r->start; p <= r->finish; p++) diff --git a/gcc/lra.c b/gcc/lra.c index fec2383..047e64f 100644 --- a/gcc/lra.c +++ b/gcc/lra.c @@ -1270,6 +1270,8 @@ static int reg_info_size; /* Common info about each register. */ struct lra_reg *lra_reg_info; +HARD_REG_SET hard_regs_spilled_into; + /* Last register value. */ static int last_reg_value; @@ -1319,6 +1321,7 @@ init_reg_info (void) for (i = 0; i < reg_info_size; i++) initialize_lra_reg_info_element (i); copy_vec.truncate (0); + CLEAR_HARD_REG_SET (hard_regs_spilled_into); }