public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Kyrill Tkachov <kyrylo.tkachov@foss.arm.com>
To: Bernd Schmidt <bschmidt@redhat.com>,
	 GCC Patches <gcc-patches@gcc.gnu.org>
Cc: Segher Boessenkool <segher@kernel.crashing.org>
Subject: Re: [PATCH][combine] PR rtl-optimization/68651 Try changing rtx from (r + r) to (r << 1) to aid recognition
Date: Tue, 15 Dec 2015 16:21:00 -0000	[thread overview]
Message-ID: <56703E12.40705@foss.arm.com> (raw)
In-Reply-To: <56702223.80702@redhat.com>

Hi Bernd,

On 15/12/15 14:22, Bernd Schmidt wrote:
> On 12/14/2015 01:25 PM, Kyrill Tkachov wrote:
>> PR 68651 is a code quality regression for GCC 5 and GCC 6 that was
>> introduced due to updated rtx costs
>> for -mcpu=cortex-a53 that affected expansion.  The costs changes were
>> correct (to the extent that rtx
>> costs have any meaning) and I think this is a deficiency in combine that
>> should be fixed.
>
> Thinking a bit more about this, I'm actually not sure that this isn't a backend problem. IMO the costs could and maybe very well should be represented such that a left shift by 1 and an add have the same cost, and the insn pattern for the 
> shift should emit the add if it is cheaper. If there are multiple ways of expressing an operation, then how it is represented in RTL is essentially irrelevant to the question of how much it costs.
>

Then for the shift pattern in the MD file we'd have to dynamically select the scheduling type depending on whether or not
the shift amount is 1 and the costs line up?
Sounds like going out of our way to work around the fact that either combine or recog does not understand equivalent forms
of instructions.

I'd be more inclined to follow your suggestion in your first reply (teaching genrecog about equivalent patterns)
However, I think that would require a bit more involved surgery in recog/combine whereas this approach
reuses the existing change_zero_ext approach which I feel is less intrusive at this stage.

The price we pay when trying these substitutions is an iteration over the rtx with FOR_EACH_SUBRTX_PTR.
recog gets called only if that iteration actually performed a substitution of x + x into x << 1.
Is that too high a price to pay? (I'm not familiar with the performance characteristics of
the FOR_EACH_SUBRTX machinery)

Thanks,
Kyrill

>
> Bernd
>

  parent reply	other threads:[~2015-12-15 16:21 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-14 12:25 Kyrill Tkachov
2015-12-15 13:22 ` Bernd Schmidt
2015-12-15 14:22 ` Bernd Schmidt
2015-12-15 14:33   ` Richard Earnshaw
2015-12-15 14:37     ` Bernd Schmidt
2015-12-15 16:21   ` Kyrill Tkachov [this message]
2015-12-16 12:18     ` Bernd Schmidt
2015-12-16 17:00       ` Kyrill Tkachov
2015-12-16 17:28         ` Jeff Law
2015-12-17  9:46           ` Kyrill Tkachov

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=56703E12.40705@foss.arm.com \
    --to=kyrylo.tkachov@foss.arm.com \
    --cc=bschmidt@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=segher@kernel.crashing.org \
    /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).