From: Ian Lance Taylor <iant@google.com>
To: Michael Matz <matz@suse.de>
Cc: gcc@gcc.gnu.org, gcc-patches@gcc.gnu.org
Subject: Re: Optimization of conditional access to globals: thread-unsafe?
Date: Fri, 26 Oct 2007 21:29:00 -0000 [thread overview]
Message-ID: <m33avxfu2i.fsf@localhost.localdomain> (raw)
In-Reply-To: <Pine.LNX.4.64.0710261836440.23011@wotan.suse.de>
Michael Matz <matz@suse.de> writes:
> Both, the assessment of far-stretchedness and these numbers seem to be
> invented ad hoc. The latter is irrelevant (it's not interesting how many
> cases there are, but how important those cases which occur are, for some
> metric, let's say performance). And the former isn't true, i.e. the
> concern is not far-stretched. For 456.hmmer for instance it is crucial
> that this transformation happens, the basic situation looks like so:
What do people think of this patch? This seems to fix the problem
case without breaking Michael's case. It basically avoids store
speculation: we don't write to a MEM unless the function
unconditionally writes to the MEM anyhow.
This is basically a public relations exercise. I doubt this
optimization is especially important, so I think it's OK to disable it
to keep people happy. Even though the optimization has been there
since gcc 3.4 and nobody noticed.
Of course this kind of thing will break again until somebody takes the
time to fully implement something like the C++0x memory model.
I haven't tested this patch.
Ian
Index: ifcvt.c
===================================================================
--- ifcvt.c (revision 128958)
+++ ifcvt.c (working copy)
@@ -2139,6 +2139,32 @@ noce_mem_write_may_trap_or_fault_p (cons
return false;
}
+/* Return whether a MEM is unconditionally set in the function
+ following TOP_BB. */
+
+static bool
+noce_mem_unconditionally_set_p (basic_block top_bb, const_rtx mem)
+{
+ basic_block dominator;
+
+ for (dominator = get_immediate_dominator (CDI_POST_DOMINATORS, top_bb);
+ dominator != NULL;
+ dominator = get_immediate_dominator (CDI_POST_DOMINATORS, dominator))
+ {
+ rtx insn;
+
+ FOR_BB_INSNS (dominator, insn)
+ {
+ if (memory_modified_in_insn_p (mem, insn))
+ return true;
+ if (modified_in_p (XEXP (mem, 0), insn))
+ return false;
+ }
+ }
+
+ return false;
+}
+
/* Given a simple IF-THEN-JOIN or IF-THEN-ELSE-JOIN block, attempt to convert
it without using conditional execution. Return TRUE if we were successful
at converting the block. */
@@ -2292,17 +2318,31 @@ noce_process_if_block (struct noce_if_in
goto success;
}
- /* Disallow the "if (...) x = a;" form (with an implicit "else x = x;")
- for optimizations if writing to x may trap or fault, i.e. it's a memory
- other than a static var or a stack slot, is misaligned on strict
- aligned machines or is read-only.
- If x is a read-only memory, then the program is valid only if we
- avoid the store into it. If there are stores on both the THEN and
- ELSE arms, then we can go ahead with the conversion; either the
- program is broken, or the condition is always false such that the
- other memory is selected. */
- if (!set_b && MEM_P (orig_x) && noce_mem_write_may_trap_or_fault_p (orig_x))
- return FALSE;
+ if (!set_b && MEM_P (orig_x))
+ {
+ /* Disallow the "if (...) x = a;" form (implicit "else x = x;")
+ for optimizations if writing to x may trap or fault,
+ i.e. it's a memory other than a static var or a stack slot,
+ is misaligned on strict aligned machines or is read-only. If
+ x is a read-only memory, then the program is valid only if we
+ avoid the store into it. If there are stores on both the
+ THEN and ELSE arms, then we can go ahead with the conversion;
+ either the program is broken, or the condition is always
+ false such that the other memory is selected. */
+ if (noce_mem_write_may_trap_or_fault_p (orig_x))
+ return FALSE;
+
+ /* Avoid store speculation: given "if (...) x = a" where x is a
+ MEM, we only want to do the store if x is always set
+ somewhere in the function. This avoids cases like
+ if (pthread_mutex_trylock(mutex))
+ ++global_variable;
+ where we only want global_variable to be changed if the mutex
+ is held. FIXME: This should ideally be expressed directly in
+ RTL somehow. */
+ if (!noce_mem_unconditionally_set_p (test_bb, orig_x))
+ return FALSE;
+ }
if (noce_try_move (if_info))
goto success;
@@ -3957,7 +3997,7 @@ dead_or_predicable (basic_block test_bb,
/* Main entry point for all if-conversion. */
static void
-if_convert (bool recompute_dominance)
+if_convert (void)
{
basic_block bb;
int pass;
@@ -3977,9 +4017,8 @@ if_convert (bool recompute_dominance)
loop_optimizer_finalize ();
free_dominance_info (CDI_DOMINATORS);
- /* Compute postdominators if we think we'll use them. */
- if (HAVE_conditional_execution || recompute_dominance)
- calculate_dominance_info (CDI_POST_DOMINATORS);
+ /* Compute postdominators. */
+ calculate_dominance_info (CDI_POST_DOMINATORS);
df_set_flags (DF_LR_RUN_DCE);
@@ -4068,7 +4107,7 @@ rest_of_handle_if_conversion (void)
if (dump_file)
dump_flow_info (dump_file, dump_flags);
cleanup_cfg (CLEANUP_EXPENSIVE);
- if_convert (false);
+ if_convert ();
}
cleanup_cfg (0);
@@ -4105,7 +4144,7 @@ gate_handle_if_after_combine (void)
static unsigned int
rest_of_handle_if_after_combine (void)
{
- if_convert (true);
+ if_convert ();
return 0;
}
@@ -4138,7 +4177,7 @@ gate_handle_if_after_reload (void)
static unsigned int
rest_of_handle_if_after_reload (void)
{
- if_convert (true);
+ if_convert ();
return 0;
}
next prev parent reply other threads:[~2007-10-26 21:24 UTC|newest]
Thread overview: 206+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <e2e108260710260541n61462585u99de9bc0617720f4@mail.gmail.com>
2007-10-26 13:45 ` Bart Van Assche
2007-10-26 14:38 ` Tomash Brechko
2007-10-26 15:50 ` Ian Lance Taylor
2007-10-26 15:51 ` Tomash Brechko
2007-10-26 16:04 ` Dave Korn
2007-10-26 16:18 ` Tomash Brechko
2007-10-26 17:06 ` Michael Matz
2007-10-26 17:54 ` Tomash Brechko
2007-10-26 17:55 ` Tomash Brechko
2007-10-28 17:08 ` Michael Matz
2007-10-28 18:06 ` Tomash Brechko
2007-10-28 18:43 ` Tomash Brechko
2007-10-29 1:29 ` Dave Korn
2007-10-29 2:05 ` David Miller
2007-10-29 2:54 ` Dave Korn
2007-10-29 3:04 ` David Miller
2007-10-29 3:08 ` David Miller
2007-10-29 4:35 ` Mark Mielke
2007-10-29 8:03 ` Tomash Brechko
2007-10-29 8:08 ` Tomash Brechko
2007-10-29 8:11 ` Andrew Pinski
2007-10-29 8:22 ` Tomash Brechko
2007-10-29 8:21 ` Eric Botcazou
2007-10-29 8:30 ` Tomash Brechko
2007-10-29 8:42 ` Eric Botcazou
2007-10-29 8:44 ` Tomash Brechko
2007-10-29 8:49 ` Tomash Brechko
2007-10-29 8:55 ` Eric Botcazou
2007-10-29 9:04 ` Tomash Brechko
2007-10-29 9:12 ` Tomash Brechko
2007-10-29 9:35 ` Tomash Brechko
2007-10-29 22:04 ` Eric Botcazou
2007-10-30 7:48 ` Tomash Brechko
2007-10-30 7:55 ` Tomash Brechko
2007-10-30 7:59 ` Eric Botcazou
2007-10-30 8:03 ` Tomash Brechko
2007-10-30 8:20 ` Tomash Brechko
2007-10-30 8:29 ` Eric Botcazou
2007-10-30 9:04 ` Tomash Brechko
2007-10-30 14:48 ` Eric Botcazou
2007-10-30 15:27 ` Tomash Brechko
2007-10-31 2:21 ` Eric Botcazou
2007-10-29 3:32 ` skaller
2007-10-29 4:32 ` David Miller
2007-10-29 4:54 ` skaller
2007-10-29 15:14 ` Michael Matz
2007-10-29 5:08 ` Darryl Miles
2007-10-29 7:43 ` David Miller
2007-10-29 12:08 ` Darryl Miles
2007-10-29 12:14 ` Robert Dewar
2007-10-29 17:04 ` skaller
2007-10-29 16:47 ` Joe Buck
2007-10-29 15:00 ` Michael Matz
2007-10-29 16:20 ` Tomash Brechko
2007-10-29 16:32 ` Tomash Brechko
2007-10-29 19:43 ` Duncan Sands
2007-10-29 20:03 ` Jack Lloyd
2007-10-29 20:52 ` Tomash Brechko
2007-10-29 20:59 ` Michael Matz
2007-10-29 21:14 ` Tomash Brechko
2007-10-26 21:29 ` Ian Lance Taylor [this message]
2007-10-26 21:39 ` Diego Novillo
2007-10-26 22:38 ` Ian Lance Taylor
2007-10-26 22:46 ` Jonathan Wakely
2007-10-26 22:56 ` Diego Novillo
2007-10-31 22:43 ` Jason Merrill
2007-10-31 22:50 ` Jason Merrill
2007-10-26 21:53 ` Daniel Jacobowitz
2007-10-26 22:20 ` Jakub Jelinek
2007-10-26 22:55 ` Ian Lance Taylor
2007-10-27 0:17 ` skaller
2007-10-27 0:26 ` David Daney
2007-10-27 0:36 ` Robert Dewar
2007-10-27 1:29 ` skaller
2007-10-27 12:51 ` Andrew Haley
2007-10-26 22:57 ` David Miller
2007-10-28 17:10 ` Michael Matz
2007-10-29 1:01 ` David Miller
2007-10-29 2:23 ` Mark Mielke
2007-10-29 15:09 ` Michael Matz
2007-10-29 15:16 ` Darryl Miles
2007-10-29 15:24 ` Michael Matz
2007-10-29 15:40 ` Darryl Miles
2007-10-29 15:16 ` Mark Mielke
2007-10-30 10:28 ` Tomash Brechko
2007-10-30 14:50 ` Ian Lance Taylor
2007-10-30 16:17 ` Tomash Brechko
2007-10-30 17:05 ` Ian Lance Taylor
2007-10-30 22:01 ` Tomash Brechko
2007-10-29 1:05 ` David Miller
2007-10-29 1:16 ` Dave Korn
2007-10-29 1:37 ` David Miller
2007-10-29 3:22 ` skaller
2007-10-29 11:54 ` Robert Dewar
2007-10-29 15:21 ` Michael Matz
2007-10-29 15:34 ` Robert Dewar
2007-10-29 15:35 ` Michael Matz
2007-10-29 15:40 ` Robert Dewar
2007-10-29 16:29 ` Joe Buck
2007-10-29 16:53 ` Robert Dewar
2007-10-26 17:10 ` skaller
2007-10-26 19:11 ` Tomash Brechko
2007-10-26 23:34 ` skaller
2007-10-27 10:54 ` Tomash Brechko
2007-10-26 15:24 ` Ian Lance Taylor
[not found] <Pine.LNX.4.64.0710281753210.23011@wotan.suse.de.suse.lists.egcs>
[not found] ` <20071028.180108.71876074.davem@davemloft.net.suse.lists.egcs>
[not found] ` <02e701c819c7$be985620$2e08a8c0@CAM.ARTIMI.COM.suse.lists.egcs>
[not found] ` <20071028.183401.197068473.davem@davemloft.net.suse.lists.egcs>
[not found] ` <20071029162032.GA10611@synopsys.com.suse.lists.egcs>
[not found] ` <47260E97.4020309@adacore.com.suse.lists.egcs>
2007-10-29 19:51 ` Andi Kleen
2007-10-29 20:00 ` Robert Dewar
2007-10-29 20:10 ` Andi Kleen
2007-10-29 20:19 ` Robert Dewar
2007-10-29 21:29 ` skaller
2007-10-29 22:07 ` Robert Dewar
2007-10-30 1:40 ` Robert Dewar
2007-10-30 6:37 ` Eric Botcazou
[not found] <e2e108260710260634q7a291337s6e66dfa25f28b68a@mail.gmail.com.suse.lists.egcs>
[not found] ` <e2e108260710260705s170a7c82udb0c9db26a408d84@mail.gmail.com.suse.lists.egcs>
[not found] ` <18210.795.425145.46885@zebedee.pink.suse.lists.egcs>
[not found] ` <e2e108260710270510j56fe188dkabe070f4c6bcbe0a@mail.gmail.com.suse.lists.egcs>
[not found] ` <87hckcpvp5.fsf@mid.deneb.enyo.de.suse.lists.egcs>
[not found] ` <e2e108260710270607u6798af5em6467bd38788f48cd@mail.gmail.com.suse.lists.egcs>
[not found] ` <87abq4ofym.fsf@mid.deneb.enyo.de.suse.lists.egcs>
[not found] ` <e2e108260710280631i405e4fd8te51ff7aa2ebece23@mail.gmail.com.suse.lists.egcs>
[not found] ` <472492F8.90700@adacore.com.suse.lists.egcs>
[not found] ` <20071028141821.GA4898@moonlight.home.suse.lists.egcs>
2007-10-29 11:57 ` Andi Kleen
2007-10-29 12:18 ` Tomash Brechko
2007-10-29 14:12 ` Andi Kleen
[not found] <e2e108260710260541n61462585u99de9bc0617720f4@mail.gmail.com.suse.lists.egcs>
[not found] ` <e2e108260710260620k2a2e21b3t1d6c052f14d36094@mail.gmail.com.suse.lists.egcs>
[not found] ` <20071026143334.GA5041@moonlight.home.suse.lists.egcs>
[not found] ` <m38x5pj3ig.fsf@localhost.localdomain.suse.lists.egcs>
[not found] ` <20071026155101.GB5041@moonlight.home.suse.lists.egcs>
[not found] ` <016201c817e9$5454edd0$2e08a8c0@CAM.ARTIMI.COM.suse.lists.egcs>
[not found] ` <20071026161739.GC5041@moonlight.home.suse.lists.egcs>
[not found] ` <Pine.LNX.4.64.0710261836440.23011@wotan.suse.de.suse.lists.egcs>
[not found] ` <m33avxfu2i.fsf@localhost.localdomain.suse.lists.egcs>
2007-10-27 17:08 ` Andi Kleen
2007-10-27 18:24 ` Ian Lance Taylor
[not found] <e2e108260710260634q7a291337s6e66dfa25f28b68a@mail.gmail.com>
2007-10-26 14:11 ` Bart Van Assche
2007-10-26 15:14 ` Andrew Haley
2007-10-26 15:18 ` Robert Dewar
2007-10-26 15:27 ` Dave Korn
2007-10-26 16:28 ` skaller
2007-10-26 16:38 ` Michael Matz
2007-10-26 17:04 ` Richard Kenner
2007-10-26 16:00 ` Samuel Tardieu
2007-10-26 17:03 ` Samuel Tardieu
2007-10-27 9:33 ` Robert Dewar
2007-10-27 13:49 ` Florian Weimer
2007-10-27 13:59 ` Samuel Tardieu
2007-10-27 14:25 ` Florian Weimer
2007-10-27 19:35 ` Andrew Haley
2007-10-27 16:25 ` Robert Dewar
2007-10-27 16:43 ` Samuel Tardieu
2007-10-27 12:47 ` Bart Van Assche
2007-10-27 13:07 ` Florian Weimer
2007-10-27 13:16 ` Bart Van Assche
2007-10-27 13:16 ` Andrew Haley
2007-10-27 13:34 ` Florian Weimer
2007-10-28 13:47 ` Bart Van Assche
2007-10-28 13:53 ` Robert Dewar
2007-10-28 15:03 ` Tomash Brechko
2007-10-28 21:19 ` Bart Van Assche
2007-10-29 3:19 ` skaller
2007-10-28 14:18 ` Andrew Haley
2007-10-28 15:07 ` Dave Korn
2007-10-28 17:29 ` Erik Trulsson
2007-10-28 17:26 ` Robert Dewar
2007-10-28 17:49 ` Erik Trulsson
2007-10-28 18:02 ` Andreas Schwab
2007-10-28 18:40 ` Dave Korn
2007-10-28 19:15 ` Erik Trulsson
2007-10-28 20:43 ` skaller
2007-10-29 5:17 ` Ross Smith
2007-10-28 17:39 ` Richard Guenther
2007-10-28 18:03 ` Erik Trulsson
2007-10-28 20:12 ` skaller
2007-10-28 23:04 ` Richard Guenther
2007-10-29 2:39 ` skaller
2007-10-29 9:52 ` Samuel Tardieu
2007-10-29 11:24 ` skaller
2007-10-29 13:57 ` Darryl Miles
2007-10-29 9:57 ` Andrew Haley
2007-10-26 16:08 ` skaller
[not found] <20071022093617.GA5073@moonlight.home.suse.lists.egcs>
[not found] ` <18204.31027.183382.838763@zebedee.pink.suse.lists.egcs>
[not found] ` <20071022105044.GB5073@moonlight.home.suse.lists.egcs>
[not found] ` <011501c8149b$b7156c20$2e08a8c0@CAM.ARTIMI.COM.suse.lists.egcs>
[not found] ` <20071022111704.GE5073@moonlight.home.suse.lists.egcs>
[not found] ` <011601c8149d$7050bea0$2e08a8c0@CAM.ARTIMI.COM.suse.lists.egcs>
[not found] ` <20071022112643.GG5073@moonlight.home.suse.lists.egcs>
[not found] ` <012501c814b2$f4623470$2e08a8c0@CAM.ARTIMI.COM.suse.lists.egcs>
[not found] ` <20071022143215.GH5073@moonlight.home.suse.lists.egcs>
[not found] ` <Pine.LNX.4.64.0710221757450.23011@wotan.suse.de.suse.lists.egcs>
[not found] ` <20071022171757.GI5073@moonlight.home.suse.lists.egcs>
[not found] ` <18204.57073.943880.741269@zebedee.pink.suse.lists.egcs>
2007-10-22 18:11 ` Andi Kleen
2007-10-21 14:55 Tomash Brechko
2007-10-21 15:26 ` Erik Trulsson
2007-10-21 16:16 ` Tomash Brechko
2007-10-21 18:51 ` Richard Guenther
2007-10-22 1:16 ` skaller
2007-10-21 23:07 ` Dave Korn
2007-10-22 1:25 ` skaller
2007-10-22 10:32 ` Dave Korn
2007-10-22 9:36 ` Tomash Brechko
2007-10-22 10:09 ` Erik Trulsson
2007-10-22 10:15 ` Robert Dewar
2007-10-23 16:53 ` Paul Brook
2007-10-22 17:59 ` skaller
2007-10-22 10:19 ` Andrew Haley
2007-10-22 10:50 ` Tomash Brechko
2007-10-22 10:54 ` Dave Korn
2007-10-22 11:10 ` Tomash Brechko
2007-10-22 11:00 ` Tomash Brechko
2007-10-22 11:07 ` Dave Korn
2007-10-22 11:17 ` Tomash Brechko
2007-10-22 11:19 ` Dave Korn
2007-10-22 11:26 ` Tomash Brechko
2007-10-22 13:53 ` Dave Korn
2007-10-22 14:32 ` Tomash Brechko
2007-10-22 16:15 ` Michael Matz
2007-10-22 16:22 ` Dave Korn
2007-10-22 17:18 ` Tomash Brechko
2007-10-22 17:33 ` Andrew Haley
2007-10-22 17:44 ` Tomash Brechko
2007-10-22 17:48 ` Andrew Haley
2007-10-22 18:00 ` Tomash Brechko
2007-10-23 9:45 ` Andrew Haley
2007-10-22 17:51 ` Dave Korn
2007-10-22 18:15 ` skaller
2007-10-22 18:26 ` Andrew Pinski
2007-10-22 11:08 ` Andrew Haley
2007-10-22 11:21 ` Tomash Brechko
2007-10-26 21:24 ` Florian Weimer
2007-10-27 18:15 ` Darryl Miles
2007-10-27 21:35 ` Dave Korn
2007-10-27 22:58 ` Darryl Miles
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=m33avxfu2i.fsf@localhost.localdomain \
--to=iant@google.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=gcc@gcc.gnu.org \
--cc=matz@suse.de \
/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).