public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Bernd Edlinger <bernd.edlinger@hotmail.de>
To: Richard Biener <richard.guenther@gmail.com>
Cc: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>
Subject: RE: [PING] [PATCH] PR58143/58227 wrong code at -O3
Date: Sun, 06 Oct 2013 12:22:00 -0000	[thread overview]
Message-ID: <DUB122-W3477DF4F29414E1E75B7AEE4120@phx.gbl> (raw)
In-Reply-To: <DUB122-W24DBE068205E2CE306F991E4260@phx.gbl>

Ping!


How I should proceed with this patch, is it OK?

The latest version was posted at: http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00234.html

Thanks,
Bernd.

>
> ping...
>
> On Wed, 4 Sep 2013 18:45:39, Bernd Edlinger wrote:
>>
>> On Tue, 3 Sep 2013 12:31:50, Richard Biener wrote:
>>> On Fri, Aug 30, 2013 at 6:43 PM, Bernd Edlinger
>>> <bernd.edlinger@hotmail.de> wrote:
>>>> Now I think this is good opportunity for a simple heuristic:
>>>>
>>>> If a statement is at loop level 1 we can move it out of the loop,
>>>> regardless of the fact, that it may invoke undefined behavior, because if it is
>>>> moved then out of any loops, and thus it cannot be an induction variable and
>>>> cause problems with aggressive loop optimizations later.
>>>
>>> VRP can still cause wrong-code issues (it's probably hard to generate a
>>> testcase though). Also a less conservative check would be to see
>>> whether we hoist _into_ loop level 0 (though we cannot check that at
>>> the point where you placed the check).
>>
>> Well, then I should better revert this heuristic again.
>>
>>>> Only statements with possible undefined behavior in nested loops can become
>>>> induction variable if lim moves them from one loop into an outer loop.
>>>>
>>>> Therefore with extremely much luck the test case will pass unmodified.
>>>> I tried it, and the patch passes bootstrap and causes zero regressions
>>>> with this heuristic.
>>>>
>>>> Ok for trunk now?
>>>
>>> Jakub mentioned another possibility - make sure the moved expression
>>> does not invoke undefined behavior by computing in an unsigned type.
>>
>> That is a possibility, but on the other hand, that would obscure the undefined
>> behavior and thus prevent other possible optimizations later.
>>
>> Another possibility would be to move the statement together with the
>> enclosing if-statement, thus really preserving the execution.
>>
>>> That said, for the sake of backporting we need a patch as simple as
>>> possible - so it would be interesting to see whether the patch without
>>> the loop 1 heuristic has any effect on say SPEC CPU 2006 performance.
>>
>> I do not have access to that test, but on the dhrystone benchmark this patch
>> has no influence whatsoever.
>>
>> Attached you'll find the latest version of my patch without the heuristic.
>> Bootstrapped on i686-pc-linux-gnu and regression tested again.
>>
>> Ok for trunk and 4.8 branch? 		 	   		  

  reply	other threads:[~2013-10-06 12:22 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-28 21:28 Bernd Edlinger
2013-08-29 10:04 ` Richard Biener
2013-08-30 16:50   ` Bernd Edlinger
2013-09-03 10:31     ` Richard Biener
2013-09-04 16:45       ` Bernd Edlinger
2013-09-16  8:27         ` [PING] " Bernd Edlinger
2013-10-06 12:22           ` Bernd Edlinger [this message]
2013-10-15  9:18             ` Richard Biener
2013-10-16 12:48               ` Michael Matz

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=DUB122-W3477DF4F29414E1E75B7AEE4120@phx.gbl \
    --to=bernd.edlinger@hotmail.de \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=richard.guenther@gmail.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).