public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Tom de Vries <Tom_deVries@mentor.com>
To: Matthew Fortune <Matthew.Fortune@mips.com>
Cc: Vladimir Makarov <vmakarov@redhat.com>,
	"gcc-patches@gcc.gnu.org"	<gcc-patches@gcc.gnu.org>,
	"Moore, Catherine" <Catherine_Moore@mentor.com>
Subject: Re: [PATCH, PR83327] Fix liveness analysis in lra for spilled-into hard regs
Date: Mon, 26 Feb 2018 13:46:00 -0000	[thread overview]
Message-ID: <96a64ae3-52b8-eb8a-7c31-8a65f3ed0d07@mentor.com> (raw)
In-Reply-To: <22DC0315F97E854EABE5B749527587D52BBCBED8@MIPSMAIL01.mipstec.com>

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

  reply	other threads:[~2018-02-26 13:46 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-16 20:39 patch to fix PR82353 Vladimir Makarov
2017-12-13 12:35 ` Tom de Vries
2017-12-14 17:01   ` Vladimir Makarov
2017-12-15 11:26     ` [PATCH, PR83327] Fix liveness analysis in lra for spilled-into hard regs Tom de Vries
2017-12-18 16:57       ` Vladimir Makarov
2018-01-08 16:52         ` Tom de Vries
2018-02-26  9:32           ` Tom de Vries
2018-02-26 11:01             ` Matthew Fortune
2018-02-26 13:46               ` Tom de Vries [this message]
2018-02-26 14:17                 ` Matthew Fortune
2018-02-28 22:18                 ` Matthew Fortune

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=96a64ae3-52b8-eb8a-7c31-8a65f3ed0d07@mentor.com \
    --to=tom_devries@mentor.com \
    --cc=Catherine_Moore@mentor.com \
    --cc=Matthew.Fortune@mips.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=vmakarov@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).