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.
prev 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: linkBe 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).