diff mbox

if-conversion of loops with conditionals containing memory loads and stores

Message ID AANLkTilFiv9bzm6E8LACu5-S7twh6w6uB2c91tgjMIAP@mail.gmail.com
State New
Headers show

Commit Message

Sebastian Pop June 22, 2010, 8:17 p.m. UTC 
Hi,

The attached patches (on top of the previous patch set) are needed to
if-convert the DCT loop kernel of FFmpeg.

0009 replaces the memory writes that are in predicated basic blocks
with a full write: "if (cond) A[i] = foo" is replaced with "A[i] =
cond ? foo : A[i]".  In order to do this, the patch replaces the call
to gimple_assign_rhs_could_trap_p with gimple_could_trap_p, as we now
have to check that the LHS of assign stmts in conditions do not trap.

In order to if-convert the DCT kernel:

  for (i = 0; i <= nCoeffs; i++)
    {
      level = block[i];
      if (level)
       {
         if (level < 0)
           {
             level = level * qmul - qadd;
           }
         else
           {
             level = level * qmul + qadd;
           }
         block[i] = level;
       }
    }

0010 proves that the accesses to an array in a loop do not trap (more
(than the original non-if-converted loop)) if they are executed at
every iteration.  In this DCT kernel, the read into block[i] happens
at every iteration, and thus the if-converted writes into block[i]
will not trap more than the original code.

The attached patches are the same as previously posted, with the
exception of this part from 0009:

(if_convertible_gimple_assign_stmt_p): Do not handle types that do
not match is_gimple_reg_type.

+  if (!is_gimple_reg_type (TREE_TYPE (lhs)))
+    return false;
+

this makes the patch pass with the extra checks added by Richi to
avoid operands of the COND_EXPR to contain memory loads or stores.

These two patches passed regstrap with BOOT_CFLAGS= -g -O3 -msse2 on
amd64-linux on top of the 8 other patches submitted separately.
Ok for trunk?

Thanks,
Sebastian Pop
--
AMD / Open Source Compiler Engineering / GNU Tools

Comments

Jeff Law June 23, 2010, 4:28 p.m. UTC | #1 
On 06/22/10 14:17, Sebastian Pop wrote:
> Hi,
>
> The attached patches (on top of the previous patch set) are needed to
> if-convert the DCT loop kernel of FFmpeg.
>
> 0009 replaces the memory writes that are in predicated basic blocks
> with a full write: "if (cond) A[i] = foo" is replaced with "A[i] =
> cond ? foo : A[i]".  In order to do this, the patch replaces the call
> to gimple_assign_rhs_could_trap_p with gimple_could_trap_p, as we now
> have to check that the LHS of assign stmts in conditions do not trap.
>    
Doesn't this run afoul of the C++0x memory model?

Jeff
Sebastian Pop June 23, 2010, 5:23 p.m. UTC | #2 
On Wed, Jun 23, 2010 at 11:28, Jeff Law <law@redhat.com> wrote:
> On 06/22/10 14:17, Sebastian Pop wrote:
>>
>> Hi,
>>
>> The attached patches (on top of the previous patch set) are needed to
>> if-convert the DCT loop kernel of FFmpeg.
>>
>> 0009 replaces the memory writes that are in predicated basic blocks
>> with a full write: "if (cond) A[i] = foo" is replaced with "A[i] =
>> cond ? foo : A[i]".  In order to do this, the patch replaces the call
>> to gimple_assign_rhs_could_trap_p with gimple_could_trap_p, as we now
>> have to check that the LHS of assign stmts in conditions do not trap.
>>
>
> Doesn't this run afoul of the C++0x memory model?
>

This still does fit in the -fmemory-model=single as discussed in
http://gcc.gnu.org/ml/gcc/2010-05/msg00171.html

I could add a separate flag for this transformation, such that it will
be easier to separate it from the rest of the tree-if-conversion pass
when implementing the -fmemory-model flag.

Sebastian Pop
--
AMD / Open Source Compiler Engineering / GNU Tools
Joseph Myers June 23, 2010, 7:04 p.m. UTC | #3 
On Wed, 23 Jun 2010, Jeff Law wrote:

> On 06/22/10 14:17, Sebastian Pop wrote:
> > Hi,
> > 
> > The attached patches (on top of the previous patch set) are needed to
> > if-convert the DCT loop kernel of FFmpeg.
> > 
> > 0009 replaces the memory writes that are in predicated basic blocks
> > with a full write: "if (cond) A[i] = foo" is replaced with "A[i] =
> > cond ? foo : A[i]".  In order to do this, the patch replaces the call
> > to gimple_assign_rhs_could_trap_p with gimple_could_trap_p, as we now
> > have to check that the LHS of assign stmts in conditions do not trap.
> >    
> Doesn't this run afoul of the C++0x memory model?

Not just the C++0x memory model, but you also need to be sure that A[i] is 
a valid pointer to writable memory - not an out-of-range or unaligned 
pointer (if A[i] is always *read*, you can be sure of that, and simply 
forming the address &A[i] means it is either valid or one past the end of 
an array), not a pointer to readonly memory (remember that in C it's valid 
to cast a pointer-to-const to pointer-to-non-const, so just because the 
target type isn't const qualified doesn't mean you can actually write to 
the memory).  You need to exclude the case that A[i] is not writable or 
not a valid pointer in some cases when cond is false (but is always 
writable and valid when cond is true).  I don't know if 
gimple_could_trap_p makes sufficient checks for this.
Sebastian Pop June 23, 2010, 8:05 p.m. UTC | #4 
On Wed, Jun 23, 2010 at 14:04, Joseph S. Myers <joseph@codesourcery.com> wrote:
> Not just the C++0x memory model, but you also need to be sure that A[i] is
> a valid pointer to writable memory - not an out-of-range or unaligned
> pointer (if A[i] is always *read*, you can be sure of that, and simply
> forming the address &A[i] means it is either valid or one past the end of
> an array),

The patch 0010 tries to prove that the data reference A[i] occurs either
in read or write mode at every iteration of the loop.

> not a pointer to readonly memory (remember that in C it's valid
> to cast a pointer-to-const to pointer-to-non-const, so just because the
> target type isn't const qualified doesn't mean you can actually write to
> the memory).

I have not checked this.  So if I understand correctly, to prove that
an array is not const qualified, one has to prove that there exists at
least one unconditional write to the array.

Thanks,
Sebastian
Jeff Law June 30, 2010, 4:57 p.m. UTC | #5 
On 06/23/10 11:23, Sebastian Pop wrote:
> On Wed, Jun 23, 2010 at 11:28, Jeff Law<law@redhat.com>  wrote:
>    
>> On 06/22/10 14:17, Sebastian Pop wrote:
>>      
>>> Hi,
>>>
>>> The attached patches (on top of the previous patch set) are needed to
>>> if-convert the DCT loop kernel of FFmpeg.
>>>
>>> 0009 replaces the memory writes that are in predicated basic blocks
>>> with a full write: "if (cond) A[i] = foo" is replaced with "A[i] =
>>> cond ? foo : A[i]".  In order to do this, the patch replaces the call
>>> to gimple_assign_rhs_could_trap_p with gimple_could_trap_p, as we now
>>> have to check that the LHS of assign stmts in conditions do not trap.
>>>
>>>        
>> Doesn't this run afoul of the C++0x memory model?
>>
>>      
> This still does fit in the -fmemory-model=single as discussed in
> http://gcc.gnu.org/ml/gcc/2010-05/msg00171.html
>    
Agreed.

> I could add a separate flag for this transformation, such that it will
> be easier to separate it from the rest of the tree-if-conversion pass
> when implementing the -fmemory-model flag.
>    
Probably wise.  Or at least something that makes it very easy to find 
this transformation so that we can disable it as needed for the 
different -fmemory-model options.

Jeff
Sebastian Pop June 30, 2010, 5:05 p.m. UTC | #6 
On Wed, Jun 30, 2010 at 11:57, Jeff Law <law@redhat.com> wrote:
> On 06/23/10 11:23, Sebastian Pop wrote:
>>
>> On Wed, Jun 23, 2010 at 11:28, Jeff Law<law@redhat.com>  wrote:
>>
>>>
>>> On 06/22/10 14:17, Sebastian Pop wrote:
>>>
>>>>
>>>> Hi,
>>>>
>>>> The attached patches (on top of the previous patch set) are needed to
>>>> if-convert the DCT loop kernel of FFmpeg.
>>>>
>>>> 0009 replaces the memory writes that are in predicated basic blocks
>>>> with a full write: "if (cond) A[i] = foo" is replaced with "A[i] =
>>>> cond ? foo : A[i]".  In order to do this, the patch replaces the call
>>>> to gimple_assign_rhs_could_trap_p with gimple_could_trap_p, as we now
>>>> have to check that the LHS of assign stmts in conditions do not trap.
>>>>
>>>>
>>>
>>> Doesn't this run afoul of the C++0x memory model?
>>>
>>>
>>
>> This still does fit in the -fmemory-model=single as discussed in
>> http://gcc.gnu.org/ml/gcc/2010-05/msg00171.html
>>
>
> Agreed.
>
>> I could add a separate flag for this transformation, such that it will
>> be easier to separate it from the rest of the tree-if-conversion pass
>> when implementing the -fmemory-model flag.
>>
>
> Probably wise.  Or at least something that makes it very easy to find this
> transformation so that we can disable it as needed for the different
> -fmemory-model options.
>

What about -ftree-if-convert-memory-writes?

Also, I see that the tree-if-conversion does not have a flag to
enable/disable it:

static bool
gate_tree_if_conversion (void)
{
  return flag_tree_vectorize != 0;
}

What about also adding and documenting the -ftree-if-convert flag?

Thanks,
Sebastian
Jeff Law July 1, 2010, 2:29 p.m. UTC | #7 
> What about -ftree-if-convert-memory-writes?
>    
Sounds reasonable.

> Also, I see that the tree-if-conversion does not have a flag to
> enable/disable it:
>
> static bool
> gate_tree_if_conversion (void)
> {
>    return flag_tree_vectorize != 0;
> }
>
> What about also adding and documenting the -ftree-if-convert flag?
>    
A good idea as well.

Thanks,
Jeff
diff mbox

Patch

From d260c90bc66d257ee5c410c05f713cb6ab0c268c Mon Sep 17 00:00:00 2001
From: Sebastian Pop <sebpop@gmail.com>
Date: Thu, 20 May 2010 16:23:17 -0500
Subject: [PATCH 10/10] Do not check whether memory references accessed in every iteration trap.

	* gimple.c (gimple_op_could_trap_p): Outlined from
	gimple_could_trap_p_1.
	(gimple_could_trap_p_1): Call gimple_op_could_trap_p.
	* gimple.h (gimple_op_could_trap_p): Declared.
	* tree-data-ref.h (same_data_refs): New.
	* tree-if-conv.c (is_true_predicate): New.
	(is_predicated): Call is_true_predicate.
	(ifcvt_memrefs_will_trap): New.
	(operations_could_trap): New.
	(ifcvt_could_trap_p): New.
	(if_convertible_gimple_assign_stmt_p): Call ifcvt_could_trap_p.
	Gets a vector of data refs.
	(if_convertible_stmt_p): Same.
	(if_convertible_loop_p): Before freeing the data refs vector,
	pass it to if_convertible_stmt_p.

	* gcc.dg/tree-ssa/ifc-5.c: New.
	* gcc.dg/vect/vect-ifcvt-18.c: XFail-ed.
	* gcc.dg/vect/vect-cond-1.c: Update pattern.
	* gcc.dg/vect/vect-cond-3.c: Same.
	* gcc.dg/vect/vect-cond-4.c: Same.
	* gcc.dg/vect/vect-cond-6.c: Same.
---
 gcc/gimple.c                            |   38 +++++---
 gcc/gimple.h                            |    1 +
 gcc/testsuite/gcc.dg/tree-ssa/ifc-5.c   |   31 +++++++
 gcc/testsuite/gcc.dg/vect/vect-cond-1.c |    2 +-
 gcc/testsuite/gcc.dg/vect/vect-cond-3.c |    2 +-
 gcc/testsuite/gcc.dg/vect/vect-cond-4.c |    2 +-
 gcc/testsuite/gcc.dg/vect/vect-cond-6.c |    1 +
 gcc/tree-data-ref.h                     |   24 +++++
 gcc/tree-if-conv.c                      |  150 +++++++++++++++++++++++++------
 9 files changed, 206 insertions(+), 45 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/ifc-5.c

diff --git a/gcc/gimple.c b/gcc/gimple.c
index 1a10f31..b2082a0 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -2365,24 +2365,14 @@  gimple_rhs_has_side_effects (const_gimple s)
   return false;
 }
 
+/* Helper for gimple_could_trap_p_1.  Return true if the operation of
+   S can trap.  */
 
-/* Helper for gimple_could_trap_p and gimple_assign_rhs_could_trap_p.
-   Return true if S can trap.  If INCLUDE_LHS is true and S is a
-   GIMPLE_ASSIGN, the LHS of the assignment is also checked.
-   Otherwise, only the RHS of the assignment is checked.  */
-
-static bool
-gimple_could_trap_p_1 (gimple s, bool include_lhs)
+bool
+gimple_op_could_trap_p (gimple s)
 {
-  unsigned i, start;
-  tree t, div = NULL_TREE;
   enum tree_code op;
-
-  start = (is_gimple_assign (s) && !include_lhs) ? 1 : 0;
-
-  for (i = start; i < gimple_num_ops (s); i++)
-    if (tree_could_trap_p (gimple_op (s, i)))
-      return true;
+  tree t, div = NULL_TREE;
 
   switch (gimple_code (s))
     {
@@ -2411,7 +2401,25 @@  gimple_could_trap_p_1 (gimple s, bool include_lhs)
     }
 
   return false;
+}
+
+/* Helper for gimple_could_trap_p and gimple_assign_rhs_could_trap_p.
+   Return true if S can trap.  If INCLUDE_LHS is true and S is a
+   GIMPLE_ASSIGN, the LHS of the assignment is also checked.
+   Otherwise, only the RHS of the assignment is checked.  */
+
+static bool
+gimple_could_trap_p_1 (gimple s, bool include_lhs)
+{
+  unsigned i, start;
+
+  start = (is_gimple_assign (s) && !include_lhs) ? 1 : 0;
+
+  for (i = start; i < gimple_num_ops (s); i++)
+    if (tree_could_trap_p (gimple_op (s, i)))
+      return true;
 
+  return gimple_op_could_trap_p (s);
 }
 
 
diff --git a/gcc/gimple.h b/gcc/gimple.h
index 210a622..371cede 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -879,6 +879,7 @@  gimple gimple_build_cond_from_tree (tree, tree, tree);
 void gimple_cond_set_condition_from_tree (gimple, tree);
 bool gimple_has_side_effects (const_gimple);
 bool gimple_rhs_has_side_effects (const_gimple);
+bool gimple_op_could_trap_p (gimple);
 bool gimple_could_trap_p (gimple);
 bool gimple_assign_rhs_could_trap_p (gimple);
 void gimple_regimplify_operands (gimple, gimple_stmt_iterator *);
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ifc-5.c b/gcc/testsuite/gcc.dg/tree-ssa/ifc-5.c
new file mode 100644
index 0000000..2ca1ac9
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/ifc-5.c
@@ -0,0 +1,31 @@ 
+/* { dg-do compile } */
+/* { dg-options "-c -O2 -ftree-vectorize -fdump-tree-ifcvt-stats" { target *-*-* } } */
+
+void
+dct_unquantize_h263_inter_c (short *block, int n, int qscale, int nCoeffs)
+{
+  int i, level, qmul, qadd;
+
+  qadd = (qscale - 1) | 1;
+  qmul = qscale << 1;
+
+  for (i = 0; i <= nCoeffs; i++)
+    {
+      level = block[i];
+      if (level)
+	{
+	  if (level < 0)
+	    {
+	      level = level * qmul - qadd;
+	    }
+	  else
+	    {
+	      level = level * qmul + qadd;
+	    }
+	  block[i] = level;
+	}
+    }
+}
+
+/* { dg-final { scan-tree-dump-times "Applying if-conversion" 1 "ifcvt" } } */
+/* { dg-final { cleanup-tree-dump "ifcvt" } } */
diff --git a/gcc/testsuite/gcc.dg/vect/vect-cond-1.c b/gcc/testsuite/gcc.dg/vect/vect-cond-1.c
index 4ee6713..888e5cb 100644
--- a/gcc/testsuite/gcc.dg/vect/vect-cond-1.c
+++ b/gcc/testsuite/gcc.dg/vect/vect-cond-1.c
@@ -52,7 +52,7 @@  int main (void)
   return 0;
 }
 
-/* { dg-final { scan-tree-dump-times "OUTER LOOP VECTORIZED" 1 "vect" { xfail vect_no_align } } } */
+/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 2 "vect" } } */
 /* { dg-final { cleanup-tree-dump "vect" } } */
 
 
diff --git a/gcc/testsuite/gcc.dg/vect/vect-cond-3.c b/gcc/testsuite/gcc.dg/vect/vect-cond-3.c
index 56cfbb2..42b9f35 100644
--- a/gcc/testsuite/gcc.dg/vect/vect-cond-3.c
+++ b/gcc/testsuite/gcc.dg/vect/vect-cond-3.c
@@ -60,7 +60,7 @@  int main (void)
   return 0;
 }
 
-/* { dg-final { scan-tree-dump-times "OUTER LOOP VECTORIZED" 1 "vect" { xfail vect_no_align } } } */
+/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 2 "vect" } } */
 /* { dg-final { cleanup-tree-dump "vect" } } */
 
 
diff --git a/gcc/testsuite/gcc.dg/vect/vect-cond-4.c b/gcc/testsuite/gcc.dg/vect/vect-cond-4.c
index c3a1585..1d1d8fc 100644
--- a/gcc/testsuite/gcc.dg/vect/vect-cond-4.c
+++ b/gcc/testsuite/gcc.dg/vect/vect-cond-4.c
@@ -57,7 +57,7 @@  int main (void)
   return 0;
 }
 
-/* { dg-final { scan-tree-dump-times "OUTER LOOP VECTORIZED" 1 "vect" { xfail vect_no_align } } } */
+/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 2 "vect" } } */
 /* { dg-final { cleanup-tree-dump "vect" } } */
 
 
diff --git a/gcc/testsuite/gcc.dg/vect/vect-cond-6.c b/gcc/testsuite/gcc.dg/vect/vect-cond-6.c
index e5e9391..7820444 100644
--- a/gcc/testsuite/gcc.dg/vect/vect-cond-6.c
+++ b/gcc/testsuite/gcc.dg/vect/vect-cond-6.c
@@ -56,5 +56,6 @@  int main ()
 }
 
 /* { dg-final { scan-tree-dump-times "OUTER LOOP VECTORIZED" 1 "vect" } } */
+/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" } } */
 /* { dg-final { cleanup-tree-dump "vect" } } */
       
diff --git a/gcc/tree-data-ref.h b/gcc/tree-data-ref.h
index eff5348..95a734a 100644
--- a/gcc/tree-data-ref.h
+++ b/gcc/tree-data-ref.h
@@ -417,6 +417,30 @@  extern void create_rdg_vertices (struct graph *, VEC (gimple, heap) *);
 extern bool dr_may_alias_p (const struct data_reference *,
 			    const struct data_reference *);
 
+/* Return true when the data references A and B are accessing the same
+   memory object with the same access functions.  */
+
+static inline bool
+same_data_refs (data_reference_p a, data_reference_p b)
+{
+  unsigned int i;
+
+  /* The references are exactly the same.  */
+  if (operand_equal_p (DR_REF (a), DR_REF (b), 0))
+    return true;
+
+  if (DR_NUM_DIMENSIONS (a) != DR_NUM_DIMENSIONS (b)
+      || !dr_may_alias_p (a, b)
+      || !operand_equal_p (DR_BASE_OBJECT (a), DR_BASE_OBJECT (b), 0))
+    return false;
+
+  for (i = 0; i < DR_NUM_DIMENSIONS (a); i++)
+    if (!eq_evolutions_p (DR_ACCESS_FN (a, i), DR_ACCESS_FN (b, i)))
+      return false;
+
+  return true;
+}
+
 /* Return true when the DDR contains two data references that have the
    same access functions.  */
 
diff --git a/gcc/tree-if-conv.c b/gcc/tree-if-conv.c
index 3ae6c85..9ee276f 100644
--- a/gcc/tree-if-conv.c
+++ b/gcc/tree-if-conv.c
@@ -349,6 +349,96 @@  if_convertible_phi_p (struct loop *loop, basic_block bb, gimple phi)
   return true;
 }
 
+/* Return true when the memory references of STMT will trap in the
+   if-converted code: i.e., when the data references of STMT are
+   executed unconditionally in the loop it is possible to say that in
+   the if-converted code there will not be more traps than in the
+   original non if-converted code.  Supposing that the data refs are
+   accessed in basic blocks predicated differently, if it is possible
+   to prove that the same data references are executed at every
+   iteration, then the if-converted code won't have more traps than
+   the original code.  */
+
+static bool
+ifcvt_memrefs_will_trap (gimple stmt, VEC (data_reference_p, heap) *refs)
+{
+  struct data_reference *a, *b;
+  unsigned int i, j;
+  tree cond = unfold_ssa_names (bb_predicate (gimple_bb (stmt)), 5);
+
+  for (i = 0; VEC_iterate (data_reference_p, refs, i, a); i++)
+    if (DR_STMT (a) == stmt)
+      {
+	for (j = 0; VEC_iterate (data_reference_p, refs, j, b); j++)
+	  if (i != j && same_data_refs (a, b))
+	    {
+	      basic_block bb;
+
+	      if (!is_predicated (gimple_bb (DR_STMT (b))))
+		{
+		  cond = boolean_true_node;
+		  break;
+		}
+
+	      bb = gimple_bb (DR_STMT (b));
+	      cond = fold_build2 (TRUTH_OR_EXPR, boolean_type_node, cond,
+				  unfold_ssa_names (bb_predicate (bb), 5));
+
+	      if (is_true_predicate (cond))
+		break;
+	    }
+
+	if (!is_true_predicate (cond))
+	  return true;
+      }
+
+  return false;
+}
+
+/* Same as gimple_could_trap_p_1, returns true when the statement S
+   could trap, but do not check the memory references.  */
+
+static bool
+operations_could_trap (gimple s)
+{
+  unsigned i;
+
+  for (i = 0; i < gimple_num_ops (s); i++)
+    {
+      tree expr = gimple_op (s, i);
+
+      switch (TREE_CODE (expr))
+	{
+	case ARRAY_REF:
+	case INDIRECT_REF:
+	case ALIGN_INDIRECT_REF:
+	case MISALIGNED_INDIRECT_REF:
+	  break;
+
+	default:
+	  if (tree_could_trap_p (expr))
+	    return true;
+	}
+    }
+
+  return gimple_op_could_trap_p (s);
+}
+
+/* Wrapper around gimple_could_trap_p refined for the needs of the
+   if-conversion.  Try to prove that the memory accesses of STMT could
+   not trap in the innermost loop containing STMT.  */
+
+static bool
+ifcvt_could_trap_p (gimple stmt, VEC (data_reference_p, heap) *refs)
+{
+  if (gimple_has_mem_ops (stmt)
+      && !ifcvt_memrefs_will_trap (stmt, refs)
+      && !operations_could_trap (stmt))
+    return false;
+
+  return gimple_could_trap_p (stmt);
+}
+
 /* Return true when STMT is if-convertible.
 
    GIMPLE_ASSIGN statement is not if-convertible if,
@@ -357,7 +447,8 @@  if_convertible_phi_p (struct loop *loop, basic_block bb, gimple phi)
    - LHS is not var decl.  */
 
 static bool
-if_convertible_gimple_assign_stmt_p (gimple stmt)
+if_convertible_gimple_assign_stmt_p (gimple stmt,
+				     VEC (data_reference_p, heap) *refs)
 {
   tree lhs = gimple_assign_lhs (stmt);
 
@@ -382,7 +473,7 @@  if_convertible_gimple_assign_stmt_p (gimple stmt)
       return false;
     }
 
-  if (gimple_could_trap_p (stmt))
+  if (ifcvt_could_trap_p (stmt, refs))
     {
       if (dump_file && (dump_flags & TDF_DETAILS))
 	fprintf (dump_file, "tree could trap...\n");
@@ -399,7 +490,7 @@  if_convertible_gimple_assign_stmt_p (gimple stmt)
    - it is a GIMPLE_LABEL or a GIMPLE_COND.  */
 
 static bool
-if_convertible_stmt_p (gimple stmt)
+if_convertible_stmt_p (gimple stmt, VEC (data_reference_p, heap) *refs)
 {
   switch (gimple_code (stmt))
     {
@@ -409,7 +500,7 @@  if_convertible_stmt_p (gimple stmt)
       return true;
 
     case GIMPLE_ASSIGN:
-      return if_convertible_gimple_assign_stmt_p (stmt);
+      return if_convertible_gimple_assign_stmt_p (stmt, refs);
 
     default:
       /* Don't know what to do with 'em so don't do anything.  */
@@ -692,6 +783,9 @@  if_convertible_loop_p (struct loop *loop)
   edge e;
   edge_iterator ei;
   basic_block exit_bb = NULL;
+  bool res = false;
+  VEC (data_reference_p, heap) *refs;
+  VEC (ddr_p, heap) *ddrs;
 
   /* Handle only innermost loop.  */
   if (!loop || loop->inner)
@@ -729,18 +823,14 @@  if_convertible_loop_p (struct loop *loop)
 
   /* Don't if-convert the loop when the data dependences cannot be
      computed: the loop won't be vectorized in that case.  */
-  {
-    VEC (data_reference_p, heap) *refs = VEC_alloc (data_reference_p, heap, 5);
-    VEC (ddr_p, heap) *ddrs = VEC_alloc (ddr_p, heap, 25);
-    bool res = compute_data_dependences_for_loop (loop, true, &refs, &ddrs);
-
-    free_data_refs (refs);
-    free_dependence_relations (ddrs);
+  refs = VEC_alloc (data_reference_p, heap, 5);
+  ddrs = VEC_alloc (ddr_p, heap, 25);
+  res = compute_data_dependences_for_loop (loop, true, &refs, &ddrs);
 
-    if (!res)
-      return false;
-  }
+  if (!res)
+    goto cleanup;
 
+  res = false;
   calculate_dominance_info (CDI_DOMINATORS);
 
   /* Allow statements that can be handled during if-conversion.  */
@@ -749,7 +839,7 @@  if_convertible_loop_p (struct loop *loop)
     {
       if (dump_file && (dump_flags & TDF_DETAILS))
 	fprintf (dump_file, "Irreducible loop\n");
-      return false;
+      goto cleanup;
     }
 
   for (i = 0; i < loop->num_nodes; i++)
@@ -757,14 +847,18 @@  if_convertible_loop_p (struct loop *loop)
       basic_block bb = ifc_bbs[i];
 
       if (!if_convertible_bb_p (loop, bb, exit_bb))
-	return false;
+	goto cleanup;
 
       if (bb_with_exit_edge_p (loop, bb))
 	exit_bb = bb;
     }
 
-  if (!predicate_bbs (loop))
-    return false;
+  res = predicate_bbs (loop);
+
+  if (!res)
+    goto cleanup;
+
+  res = false;
 
   for (i = 0; i < loop->num_nodes; i++)
     {
@@ -773,21 +867,23 @@  if_convertible_loop_p (struct loop *loop)
 
       for (itr = gsi_start_phis (bb); !gsi_end_p (itr); gsi_next (&itr))
 	if (!if_convertible_phi_p (loop, bb, gsi_stmt (itr)))
-	  return false;
+	  goto cleanup;
 
-      /* For non predicated BBs, don't check their statements.  */
-      if (!is_predicated (bb))
-	continue;
-
-      for (itr = gsi_start_bb (bb); !gsi_end_p (itr); gsi_next (&itr))
-	if (!if_convertible_stmt_p (gsi_stmt (itr)))
-	  return false;
+      /* Check the if-convertibility of statements in predicated BBs.  */
+      if (is_predicated (bb))
+	for (itr = gsi_start_bb (bb); !gsi_end_p (itr); gsi_next (&itr))
+	  if (!if_convertible_stmt_p (gsi_stmt (itr), refs))
+	    goto cleanup;
     }
 
+  res = true;
   if (dump_file)
     fprintf (dump_file, "Applying if-conversion\n");
 
-  return true;
+ cleanup:
+  free_data_refs (refs);
+  free_dependence_relations (ddrs);
+  return res;
 }
 
 /* Basic block BB has two predecessors.  Using predecessor's bb
-- 
1.7.0.4