On 10/16/12 20:33, Ian Lance Taylor wrote:
On Tue, Oct 16, 2012 at 4:54 PM, Aldy Hernandez <al...@redhat.com> wrote:
On 10/16/12 13:28, Jakub Jelinek wrote:
location of considered if-conversion on mem
if (some_condition)
call ();
mem = something;
Are we not currently being conservative in the current code and returning
false once we see a volatile or a non-const call?
Jakub's point is that in his example the call will not be a
post-dominator of the block. it's a good point. Would you mind
trying to create a test case along those lines?
Ah, I see.
Well, I can't come up with a testcase that fits into our testing
infrastructure because of the aforementioned problems catching an
addition of zero. I could scan the assembly, but anything I could come
up with is not only back-end specific, but extremely fragile.
I am attaching a testcase that I have used to (manually) test the patch
below. The relevant parts are in the attached testcase is:
...
TOP_BB
outerfunc (innerfunc ());
if (flag_for_funky)
funky();
dont_write += 666;
My approach is, once we find the DONT_WRITE block, we generate a list of
all the blocks at are post-dominated by it, and if there is a
volatile/call that is between TOP_BB and the write, then a volatile/call
will occur and we can't perform the speculation.
This works with the testcase at hand, though probably a bit inefficient.
For one, we could calculate the dominance info earlier in the pass,
though I prefer calculating it only if we're actually considering
speculating. I'm open to suggestions.
I'm running into a weird verify_dominators() problem during bootstrap--
which is odd, because I'm forcing a recompute of the dominance tree.
But I wanted to post this to make sure I'm at least poking in the right
direction.
Thoughts?
PR rtl-optimization/54900
* ifcvt.c (volatile_or_non_const_call): New.
(noce_can_store_speculate_p): Handle intervening calls.
diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
index 5654c66..c615efd 100644
--- a/gcc/ifcvt.c
+++ b/gcc/ifcvt.c
@@ -2388,6 +2388,16 @@ noce_mem_write_may_trap_or_fault_p (const_rtx mem)
return false;
}
+/* Return TRUE if INSN is a volatile insn or a non const call. */
+
+static inline bool
+volatile_or_non_const_call (rtx insn)
+{
+ return (INSN_P (insn)
+ && (volatile_insn_p (PATTERN (insn))
+ || (CALL_P (insn) && (!RTL_CONST_CALL_P (insn)))));
+}
+
/* Return whether we can use store speculation for MEM. TOP_BB is the
basic block above the conditional block where we are considering
doing the speculative store. We look for whether MEM is set
@@ -2410,13 +2420,47 @@ noce_can_store_speculate_p (basic_block top_bb,
const_rtx mem)
have to stop looking. Even if the MEM is set later in
the function, we still don't want to set it
unconditionally before the barrier. */
- if (INSN_P (insn)
- && (volatile_insn_p (PATTERN (insn))
- || (CALL_P (insn) && (!RTL_CONST_CALL_P (insn)))))
+ if (volatile_or_non_const_call (insn))
return false;
+ /* We can speculate if the MEM will be set on every path out
+ of TOP_BB, but we must be careful that there is no
+ intervening call/volatile between TOP_BB and the set.
+
+ For the intervening call/volatile case, We are looking
+ for something akin to:
+
+ location of considered if-conversion on mem
+ if (condition)
+ call ();
+ mem = something; */
if (memory_must_be_modified_in_insn_p (mem, insn))
- return true;
+ {
+ if (!dom_info_available_p (CDI_DOMINATORS))
+ calculate_dominance_info (CDI_DOMINATORS);
+
+ /* Get all blocks that are post-dominated by the SET. */
+ VEC (basic_block, heap) *bbs =
+ get_all_dominated_blocks (CDI_POST_DOMINATORS,
+ BLOCK_FOR_INSN (insn));
+ while (VEC_length (basic_block, bbs))
+ {
+ rtx call;
+ basic_block bb = VEC_pop (basic_block, bbs);
+
+ /* If there is an intervening volatile/call between
+ TOP_BB and the SET of MEM, bail. */
+ FOR_BB_INSNS (bb, call)
+ if (volatile_or_non_const_call (call)
+ && dominated_by_p (CDI_DOMINATORS,
+ BLOCK_FOR_INSN (call),
+ top_bb))
+ return false;
+ }
+ VEC_free (basic_block, heap, bbs);
+ return true;
+ }
+
if (modified_in_p (XEXP (mem, 0), insn))
return false;
/* Compile with -O2. */
/* There shouldn't be a an unconditional write to dont_write in main(). */
extern int printf(const char *format, ...);
int flag = 1;
int flag_for_funky;
int stuff;
int *stuffp = &stuff;
int dont_write;
extern void funky();
void outerfunc (p1) {
*stuffp = 0;
}
int innerfunc ()
{
if (flag)
return 0;
++dont_write;
return 0;
}
void main ()
{
outerfunc (innerfunc ());
if (flag_for_funky)
funky();
dont_write += 666;
}
