Tom de Vries <tom_devr...@mentor.com> writes: > On 02/26/2018 12:00 PM, Matthew Fortune wrote: > > Tom de Vries <tom_devr...@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