public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
From: "amker at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug tree-optimization/50955] [4.7 Regression] IVopts incorrectly rewrite the address of a global memory access into a local form.
Date: Fri, 28 Aug 2015 09:16:00 -0000	[thread overview]
Message-ID: <bug-50955-4-ORCn52zqAo@http.gcc.gnu.org/bugzilla/> (raw)
In-Reply-To: <bug-50955-4@http.gcc.gnu.org/bugzilla/>

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=50955

amker at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |amker at gcc dot gnu.org

--- Comment #21 from amker at gcc dot gnu.org ---
For the record, here are some findings for this issue.

1) It isn't well-defined in terms of C standard to do below transformation in
GCC:
     a) convert pointers to unsigned integers;
     b) do arithmetic computations on converted unsigned integers;
        like: (szietype)ptr1 - (sizetype)ptr2 - 1 + (sizetype)ptr2
     c) convert result integer back to pointer;
     d) access memory using the result pointer;

2) GCC may still generates such transformation.  As said it's too conservative
to follow C standard strictly in this aspect.

3) When doing such transformation, we need to guarantee TMR.base is derived
correctly.  Otherwise GCC's alias analysis would be confused and results in
wrong code.

4) The problem with IVOPT is the approach it takes to derive TMR.base. 
Firstly, it depends on iv_cand.base_object to derive correct base for TMR. 
This is why GCC skips candidate with base_object for any iv_use with different
base_object.

5) when approach in 4) fails, it falls back to primitive approach by simply
setting TMR.base to any pointer in the result address expression.  Of course,
it could be wrong.  This is why PR50955 is triggered in the first place.

With these understandings, I can see two possible fixes here.

Fix 1) Just like Richard has committed, we not only check different base_object
on address type iv_uses, but also on generic type iv_uses.  This is too
conservative and we need to resolve performance regressions reported in
PR52272.

Fix 2) We need to improve how TMR.base is derived in IVOPTs.  New approach is
instead of iv_cand.base_object, we can use iv_use.base_object.  That is, check
the result address expression to see if it includes iv_use.base_object.  If
yes, we can set TMR.base to it; otherwise we can reject the iv_cand.  (actually
we can create a temp ssa_var with iv_use.base_object's pointer information
copied)

There is one concern about Fix 2).  The get_computation_cost_at needs to be
re-implemented to compute the result address expression like
get_computation_aff does.

Also Fix 2) doesn't give more benefit comparing to proposal patch in PR52272. 
Though that proposal has its own problem.


      parent reply	other threads:[~2015-08-28  9:16 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-02  5:12 [Bug tree-optimization/50955] New: " duyuehai at gmail dot com
2011-11-02  5:19 ` [Bug tree-optimization/50955] " duyuehai at gmail dot com
2011-11-02  9:42 ` [Bug tree-optimization/50955] [4.7 Regression] " rguenth at gcc dot gnu.org
2011-11-02 12:50 ` rguenth at gcc dot gnu.org
2011-11-02 13:10 ` rguenth at gcc dot gnu.org
2011-11-02 13:33 ` rguenth at gcc dot gnu.org
2011-11-03  6:25 ` duyuehai at gmail dot com
2011-11-03  7:52 ` rguenther at suse dot de
2011-11-03  8:07 ` rakdver at kam dot mff.cuni.cz
2011-11-03  8:19 ` rguenth at gcc dot gnu.org
2011-12-06 13:56 ` rguenth at gcc dot gnu.org
2012-01-31 10:51 ` rguenth at gcc dot gnu.org
2012-01-31 14:24 ` rguenth at gcc dot gnu.org
2012-02-06 13:43 ` rguenth at gcc dot gnu.org
2012-02-06 13:44 ` rguenth at gcc dot gnu.org
2013-08-16  4:19 ` pinskia at gcc dot gnu.org
2013-08-16  7:02 ` pinskia at gcc dot gnu.org
2013-08-28  8:33 ` rguenther at suse dot de
2013-12-18  9:42 ` rguenther at suse dot de
2013-12-18 10:24 ` amker.cheng at gmail dot com
2013-12-19  9:57 ` rguenth at gcc dot gnu.org
2015-08-28  9:16 ` amker at gcc dot gnu.org [this message]

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=bug-50955-4-ORCn52zqAo@http.gcc.gnu.org/bugzilla/ \
    --to=gcc-bugzilla@gcc.gnu.org \
    --cc=gcc-bugs@gcc.gnu.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).