This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
RE: [PATCH, PR83327] Fix liveness analysis in lra for spilled-into hard regs
- From: Matthew Fortune <Matthew dot Fortune at mips dot com>
- To: Tom de Vries <Tom_deVries at mentor dot com>
- Cc: Vladimir Makarov <vmakarov at redhat dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, "Moore, Catherine" <Catherine_Moore at mentor dot com>
- Date: Wed, 28 Feb 2018 21:39:08 +0000
- Subject: RE: [PATCH, PR83327] Fix liveness analysis in lra for spilled-into hard regs
- Authentication-results: sourceware.org; auth=none
- References: <5794f39e-42ef-66da-e7f5-270ebc54cdf2@redhat.com> <98df25f2-0282-7eca-f6d9-c320c023ec0d@mentor.com> <5ae37149-3559-8221-fc74-68f4e90ff4de@redhat.com> <12e6eeea-ba32-bdcf-1e42-6480b0bbad32@mentor.com> <41dbb89c-0592-e474-849a-f6928a6135c4@redhat.com> <189821d4-1494-e47e-beeb-76a4219a206b@mentor.com> <7739e43f-3525-3114-c792-54d5a384bebe@mentor.com> <22DC0315F97E854EABE5B749527587D52BBCBED8@MIPSMAIL01.mipstec.com> <96a64ae3-52b8-eb8a-7c31-8a65f3ed0d07@mentor.com>
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