Fix if-conversion pass for dead type-unsafe code

Fix if-conversion pass for dead type-unsafe code

[Tom de Vries]

[-- Attachment #1: Type: text/plain, Size: 2219 bytes --]

Steven,

this patch fixes:
- PR62004 (the if-conversion pass part, the tail-merge part is still todo), and
- PR62030.

In both cases, a valid program with a dead type-unsafe access is transformed by 
the if-conversion pass into an invalid program with a live type-unsafe access.

The transformation done by the if-conversion pass that suffers from this problem 
is if-merging, replacing the if-then-else with the if-block or the then then-block.

The patch fixes this problem by detecting when the if-block and the then-block 
are treated differently by alias analysis, and preventing the optimization in 
that case.

This part of the patch fixes PR62004.
...
@@ -2583,7 +2631,7 @@ noce_process_if_block (struct noce_if_info *if_info)

    /* Look and see if A and B are really the same.  Avoid creating silly
       cmove constructs that no one will fix up later.  */
-  if (rtx_equal_p (a, b))
+  if (rtx_interchangeable_p (a, b))
      {
        /* If we have an INSN_B, we don't have to create any new rtl.  Just
	 move the instruction that we already have.  If we don't have an
...

This part of the patch fixes PR62030:
...
@@ -2517,7 +2565,7 @@ noce_process_if_block (struct noce_if_info *if_info)
	  || BLOCK_FOR_INSN (insn_b) != BLOCK_FOR_INSN (if_info->cond_earliest)
	  || !NONJUMP_INSN_P (insn_b)
	  || (set_b = single_set (insn_b)) == NULL_RTX
-         || ! rtx_equal_p (x, SET_DEST (set_b))
+         || ! rtx_interchangeable_p (x, SET_DEST (set_b))
	  || ! noce_operand_ok (SET_SRC (set_b))
	  || reg_overlap_mentioned_p (x, SET_SRC (set_b))
	  || modified_between_p (SET_SRC (set_b), insn_b, jump)
...

I've added the other fixes after review of the if-conversion pass for the same 
problem, I hope this is complete (well, at least for the if-conversion pass. I 
wonder if cross-jumping suffers from the same problem).

The PR62030 test-case fails with trunk on MIPS. The PR62004 testcase fails with 
4.8 on x86_64. But I think the problem exists in trunk, 4.9 and 4.8, it's just 
hard to trigger.

Bootstrapped and reg-tested on x86_64, trunk. No issue found other than 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62060 , which looks like a 
test-case issue.

OK for trunk, 4.9, 4.8?

Thanks,
- Tom

[-- Attachment #2: 0001-Fix-for-PR62004-and-PR62030.patch --]
[-- Type: text/x-patch, Size: 7157 bytes --]

2014-08-07  Tom de Vries  <tom@codesourcery.com>

	* ifcvt.c (mem_alias_equal_p, rtx_interchangeable_p): New function.
	(noce_try_move, noce_process_if_block): Use rtx_interchangeable_p.
	(noce_try_cmove_arith): Use mem_alias_equal_p.

	* gcc.dg/pr62004.c: New test.
	* gcc.dg/pr62030.c: Same.
	* gcc.target/mips/pr62030-octeon.c: Same.

diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
index faf9b30..6da3e0f 100644
--- a/gcc/ifcvt.c
+++ b/gcc/ifcvt.c
@@ -306,6 +306,52 @@ block_fallthru (basic_block bb)
 
   return (e) ? e->dest : NULL_BLOCK;
 }
+
+/* Return true if MEMs A and B are treated equal by alias analysis.  */
+
+static bool
+mem_alias_equal_p (const_rtx a, const_rtx b)
+{
+  gcc_assert (GET_CODE (a) == MEM && GET_CODE (b) == MEM);
+  
+  if (MEM_ADDR_SPACE (a) != MEM_ADDR_SPACE (b))
+    return false;
+
+  if (flag_strict_aliasing
+      && MEM_ALIAS_SET (a) != MEM_ALIAS_SET (b))
+    return false;
+
+  if (MEM_EXPR (a) != MEM_EXPR (b))
+    return false;
+
+  if (MEM_OFFSET_KNOWN_P (a) != MEM_OFFSET_KNOWN_P (b))
+    return false;
+
+  if (MEM_OFFSET_KNOWN_P (a)
+      && MEM_OFFSET (a) != MEM_OFFSET (b))
+    return false;
+
+  return true;
+}
+
+static bool
+rtx_interchangeable_p (const_rtx a, const_rtx b)
+{
+  if (!rtx_equal_p (a, b))
+    return false;
+
+  if (GET_CODE (a) != MEM)
+    return true;
+  
+  /* A dead type-unsafe memory reference is legal, but a live type-unsafe memory
+     reference is not.  Interchanging a dead type-unsafe memory reference with
+     a live type-safe one creates a live type-unsafe memory reference, in other
+     words, it makes the program illegal.  So we check whether the two memory
+     references have equal tbaa properties.  */
+
+  return mem_alias_equal_p (a, b);
+}
+
 \f
 /* Go through a bunch of insns, converting them to conditional
    execution format if possible.  Return TRUE if all of the non-note
@@ -1034,6 +1080,9 @@ noce_try_move (struct noce_if_info *if_info)
       || (rtx_equal_p (if_info->a, XEXP (cond, 1))
 	  && rtx_equal_p (if_info->b, XEXP (cond, 0))))
     {
+      if (!rtx_interchangeable_p (if_info->a, if_info->b))
+	return FALSE;
+
       y = (code == EQ) ? if_info->a : if_info->b;
 
       /* Avoid generating the move if the source is the destination.  */
@@ -1537,14 +1586,13 @@ noce_try_cmove_arith (struct noce_if_info *if_info)
   int insn_cost;
   enum rtx_code code;
 
-  /* A conditional move from two memory sources is equivalent to a
-     conditional on their addresses followed by a load.  Don't do this
-     early because it'll screw alias analysis.  Note that we've
-     already checked for no side effects.  */
-  /* ??? FIXME: Magic number 5.  */
+  /* A conditional move from two memory sources is equivalent to a conditional
+     on their addresses followed by a load.  Don't do this if it'll screw alias
+     analysis.  Note that we've already checked for no side effects.  */
   if (cse_not_expected
       && MEM_P (a) && MEM_P (b)
-      && MEM_ADDR_SPACE (a) == MEM_ADDR_SPACE (b)
+      && mem_alias_equal_p (a, b)
+      /* ??? FIXME: Magic number 5.  */
       && if_info->branch_cost >= 5)
     {
       enum machine_mode address_mode = get_address_mode (a);
@@ -2504,7 +2552,7 @@ noce_process_if_block (struct noce_if_info *if_info)
       if (! insn_b
 	  || insn_b != last_active_insn (else_bb, FALSE)
 	  || (set_b = single_set (insn_b)) == NULL_RTX
-	  || ! rtx_equal_p (x, SET_DEST (set_b)))
+	  || ! rtx_interchangeable_p (x, SET_DEST (set_b)))
 	return FALSE;
     }
   else
@@ -2517,7 +2565,7 @@ noce_process_if_block (struct noce_if_info *if_info)
 	  || BLOCK_FOR_INSN (insn_b) != BLOCK_FOR_INSN (if_info->cond_earliest)
 	  || !NONJUMP_INSN_P (insn_b)
 	  || (set_b = single_set (insn_b)) == NULL_RTX
-	  || ! rtx_equal_p (x, SET_DEST (set_b))
+	  || ! rtx_interchangeable_p (x, SET_DEST (set_b))
 	  || ! noce_operand_ok (SET_SRC (set_b))
 	  || reg_overlap_mentioned_p (x, SET_SRC (set_b))
 	  || modified_between_p (SET_SRC (set_b), insn_b, jump)
@@ -2583,7 +2631,7 @@ noce_process_if_block (struct noce_if_info *if_info)
 
   /* Look and see if A and B are really the same.  Avoid creating silly
      cmove constructs that no one will fix up later.  */
-  if (rtx_equal_p (a, b))
+  if (rtx_interchangeable_p (a, b))
     {
       /* If we have an INSN_B, we don't have to create any new rtl.  Just
 	 move the instruction that we already have.  If we don't have an
diff --git a/gcc/testsuite/gcc.dg/pr62004.c b/gcc/testsuite/gcc.dg/pr62004.c
new file mode 100644
index 0000000..7292c4f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr62004.c
@@ -0,0 +1,47 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -fno-tree-tail-merge" } */
+
+struct node
+{ 
+  struct node *next; 
+  struct node *prev; 
+};
+
+struct node node;
+
+struct head
+{
+  struct node *first;
+};
+
+struct head heads[5];
+
+int k = 2;
+
+struct head *head = &heads[2];
+
+int
+main ()
+{
+  struct node *p;
+
+  node.next = (void*)0;
+
+  node.prev = (void *)head;
+
+  head->first = &node;
+
+  struct node *n = head->first;
+
+  struct head *h = &heads[k];
+
+  heads[2].first = n->next;
+
+  if ((void*)n->prev == (void *)h)
+    p = h->first;
+  else
+    /* Dead tbaa-unsafe load from ((struct node *)&heads[2])->next.  */
+    p = n->prev->next;
+
+  return !(p == (void*)0);
+}
diff --git a/gcc/testsuite/gcc.dg/pr62030.c b/gcc/testsuite/gcc.dg/pr62030.c
new file mode 100644
index 0000000..a16a970
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr62030.c
@@ -0,0 +1,50 @@
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+extern void abort (void);
+
+struct node
+{ 
+  struct node *next; 
+  struct node *prev; 
+};
+
+struct node node;
+
+struct head
+{
+  struct node *first;
+};
+
+struct head heads[5];
+
+int k = 2;
+
+struct head *head = &heads[2];
+
+static int __attribute__((noinline))
+foo (void)
+{
+  node.prev = (void *)head;
+  head->first = &node;
+
+  struct node *n = head->first;
+  struct head *h = &heads[k];
+  struct node *next = n->next;
+
+  if (n->prev == (void *)h)
+    h->first = next;
+  else
+    n->prev->next = next;
+
+  n->next = h->first;
+  return n->next == &node;
+}
+
+int
+main (void)
+{
+  if (foo ())
+    abort ();
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/mips/pr62030-octeon.c b/gcc/testsuite/gcc.target/mips/pr62030-octeon.c
new file mode 100644
index 0000000..2f86f5e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/pr62030-octeon.c
@@ -0,0 +1,50 @@
+/* { dg-do run } */
+/* { dg-options "-march=octeon" } */
+
+extern void abort (void);
+
+struct node
+{ 
+  struct node *next; 
+  struct node *prev; 
+};
+
+struct node node;
+
+struct head
+{
+  struct node *first;
+};
+
+struct head heads[5];
+
+int k = 2;
+
+struct head *head = &heads[2];
+
+static int __attribute__((noinline))
+foo (void)
+{
+  node.prev = (void *)head;
+  head->first = &node;
+
+  struct node *n = head->first;
+  struct head *h = &heads[k];
+  struct node *next = n->next;
+
+  if (n->prev == (void *)h)
+    h->first = next;
+  else
+    n->prev->next = next;
+
+  n->next = h->first;
+  return n->next == &node;
+}
+
+int
+main (void)
+{
+  if (foo ())
+    abort ();
+  return 0;
+}
-- 
1.9.1

Re: Fix if-conversion pass for dead type-unsafe code

[Richard Biener]

On Fri, 8 Aug 2014, Tom de Vries wrote:

> Steven,
> 
> this patch fixes:
> - PR62004 (the if-conversion pass part, the tail-merge part is still todo),
> and
> - PR62030.
> 
> In both cases, a valid program with a dead type-unsafe access is transformed
> by the if-conversion pass into an invalid program with a live type-unsafe
> access.
> 
> The transformation done by the if-conversion pass that suffers from this
> problem is if-merging, replacing the if-then-else with the if-block or the
> then then-block.
> 
> The patch fixes this problem by detecting when the if-block and the then-block
> are treated differently by alias analysis, and preventing the optimization in
> that case.
> 
> This part of the patch fixes PR62004.
> ...
> @@ -2583,7 +2631,7 @@ noce_process_if_block (struct noce_if_info *if_info)
> 
>    /* Look and see if A and B are really the same.  Avoid creating silly
>       cmove constructs that no one will fix up later.  */
> -  if (rtx_equal_p (a, b))
> +  if (rtx_interchangeable_p (a, b))
>      {
>        /* If we have an INSN_B, we don't have to create any new rtl.  Just
> 	 move the instruction that we already have.  If we don't have an
> ...
> 
> This part of the patch fixes PR62030:
> ...
> @@ -2517,7 +2565,7 @@ noce_process_if_block (struct noce_if_info *if_info)
> 	  || BLOCK_FOR_INSN (insn_b) != BLOCK_FOR_INSN
> (if_info->cond_earliest)
> 	  || !NONJUMP_INSN_P (insn_b)
> 	  || (set_b = single_set (insn_b)) == NULL_RTX
> -         || ! rtx_equal_p (x, SET_DEST (set_b))
> +         || ! rtx_interchangeable_p (x, SET_DEST (set_b))
> 	  || ! noce_operand_ok (SET_SRC (set_b))
> 	  || reg_overlap_mentioned_p (x, SET_SRC (set_b))
> 	  || modified_between_p (SET_SRC (set_b), insn_b, jump)
> ...
> 
> I've added the other fixes after review of the if-conversion pass for the same
> problem, I hope this is complete (well, at least for the if-conversion pass. I
> wonder if cross-jumping suffers from the same problem).
> 
> The PR62030 test-case fails with trunk on MIPS. The PR62004 testcase fails
> with 4.8 on x86_64. But I think the problem exists in trunk, 4.9 and 4.8, it's
> just hard to trigger.
> 
> Bootstrapped and reg-tested on x86_64, trunk. No issue found other than
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62060 , which looks like a
> test-case issue.
> 
> OK for trunk, 4.9, 4.8?

Maybe instead of a new mem_alias_equal_p simply compare MEM_ATTRs
with mem_attrs_eq_p?  Note that

+  if (flag_strict_aliasing
+      && MEM_ALIAS_SET (a) != MEM_ALIAS_SET (b))
+    return false;

looks wrong as it doesn't treat ALIAS_SET_MEMORY_BARRIER specially.
Also note that PRE already does sth similar with using exp_equiv_p
and passing 'true' for the 'for_gcse' argument.  In fact
using exp_equiv_p would likely improve if-conversion if used
in place of rtx_equal_p?

Thanks,
Richard.

Re: Fix if-conversion pass for dead type-unsafe code

[Tom de Vries]

[-- Attachment #1: Type: text/plain, Size: 5028 bytes --]

On 08-08-14 13:36, Richard Biener wrote:
> On Fri, 8 Aug 2014, Tom de Vries wrote:
>
>> Steven,
>>
>> this patch fixes:
>> - PR62004 (the if-conversion pass part, the tail-merge part is still todo),
>> and
>> - PR62030.
>>
>> In both cases, a valid program with a dead type-unsafe access is transformed
>> by the if-conversion pass into an invalid program with a live type-unsafe
>> access.
>>
>> The transformation done by the if-conversion pass that suffers from this
>> problem is if-merging, replacing the if-then-else with the if-block or the
>> then then-block.
>>
>> The patch fixes this problem by detecting when the if-block and the then-block
>> are treated differently by alias analysis, and preventing the optimization in
>> that case.
>>
>> This part of the patch fixes PR62004.
>> ...
>> @@ -2583,7 +2631,7 @@ noce_process_if_block (struct noce_if_info *if_info)
>>
>>     /* Look and see if A and B are really the same.  Avoid creating silly
>>        cmove constructs that no one will fix up later.  */
>> -  if (rtx_equal_p (a, b))
>> +  if (rtx_interchangeable_p (a, b))
>>       {
>>         /* If we have an INSN_B, we don't have to create any new rtl.  Just
>> 	 move the instruction that we already have.  If we don't have an
>> ...
>>
>> This part of the patch fixes PR62030:
>> ...
>> @@ -2517,7 +2565,7 @@ noce_process_if_block (struct noce_if_info *if_info)
>> 	  || BLOCK_FOR_INSN (insn_b) != BLOCK_FOR_INSN
>> (if_info->cond_earliest)
>> 	  || !NONJUMP_INSN_P (insn_b)
>> 	  || (set_b = single_set (insn_b)) == NULL_RTX
>> -         || ! rtx_equal_p (x, SET_DEST (set_b))
>> +         || ! rtx_interchangeable_p (x, SET_DEST (set_b))
>> 	  || ! noce_operand_ok (SET_SRC (set_b))
>> 	  || reg_overlap_mentioned_p (x, SET_SRC (set_b))
>> 	  || modified_between_p (SET_SRC (set_b), insn_b, jump)
>> ...
>>
>> I've added the other fixes after review of the if-conversion pass for the same
>> problem, I hope this is complete (well, at least for the if-conversion pass.

Hmm, I see now that in noce_try_cmove_arith a new mem is generated, and merging 
of (some of the) mem attributes is done, so that part doesn't need fixing for 
4.8/4.9:
...
   /* If we're handling a memory for above, emit the load now.  */
   if (is_mem)
     {
       tmp = gen_rtx_MEM (GET_MODE (if_info->x), target);

       /* Copy over flags as appropriate.  */
       if (MEM_VOLATILE_P (if_info->a) || MEM_VOLATILE_P (if_info->b))
         MEM_VOLATILE_P (tmp) = 1;
       if (MEM_ALIAS_SET (if_info->a) == MEM_ALIAS_SET (if_info->b))
         set_mem_alias_set (tmp, MEM_ALIAS_SET (if_info->a));
       set_mem_align (tmp,
                      MIN (MEM_ALIGN (if_info->a), MEM_ALIGN (if_info->b)));

       gcc_assert (MEM_ADDR_SPACE (if_info->a) == MEM_ADDR_SPACE (if_info->b));
       set_mem_addr_space (tmp, MEM_ADDR_SPACE (if_info->a));

       noce_emit_move_insn (if_info->x, tmp);
     }
...
Dropped in attached patch.

>> I
>> wonder if cross-jumping suffers from the same problem).
>>

Cross-jumping uses merge_memattrs.

>> The PR62030 test-case fails with trunk on MIPS. The PR62004 testcase fails
>> with 4.8 on x86_64. But I think the problem exists in trunk, 4.9 and 4.8, it's
>> just hard to trigger.
>>
>> Bootstrapped and reg-tested on x86_64, trunk. No issue found other than
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62060 , which looks like a
>> test-case issue.
>>
>> OK for trunk, 4.9, 4.8?
>
> Maybe instead of a new mem_alias_equal_p simply compare MEM_ATTRs
> with mem_attrs_eq_p?

I propose to fix it this way (as attached) on 4.8/4.9/trunk, and maybe do a more 
efficient handling on trunk as a follow-up patch.

I'll put this through bootstrap/test again.

>  Note that
>
> +  if (flag_strict_aliasing
> +      && MEM_ALIAS_SET (a) != MEM_ALIAS_SET (b))
> +    return false;
>
> looks wrong as it doesn't treat ALIAS_SET_MEMORY_BARRIER specially.

Ah, I see.

> Also note that PRE already does sth similar with using exp_equiv_p
> and passing 'true' for the 'for_gcse' argument.  In fact
> using exp_equiv_p would likely improve if-conversion if used
> in place of rtx_equal_p?
>

Interesting. I've noticed btw that exp_equiv_p does not use mem_attrs_eq_p, but 
a mem-attrs pointer-equal comparison:
...
	  /* Can't merge two expressions in different alias sets, since we
	     can decide that the expression is transparent in a block when
	     it isn't, due to it being set with the different alias set.

	     Also, can't merge two expressions with different MEM_ATTRS.
	     They could e.g. be two different entities allocated into the
	     same space on the stack (see e.g. PR25130).  In that case, the
	     MEM addresses can be the same, even though the two MEMs are
	     absolutely not equivalent.

	     But because really all MEM attributes should be the same for
	     equivalent MEMs, we just use the invariant that MEMs that have
	     the same attributes share the same mem_attrs data structure.  */
	  if (MEM_ATTRS (x) != MEM_ATTRS (y))
	    return 0;
...

Thanks,
- Tom


[-- Attachment #2: 0001-Conservative-fix-for-PR62004-PR62030.patch --]
[-- Type: text/x-patch, Size: 6642 bytes --]

2014-08-08  Tom de Vries  <tom@codesourcery.com>

	PR rtl-optimization/62004
	PR rtl-optimization/62030
	* ifcvt.c (rtx_interchangeable_p): New function.
	(noce_try_move, noce_process_if_block): Use rtx_interchangeable_p.
	* emit-rtl.c (mem_attrs_eq_p): Remove static.
	* emit-rtl.h (mem_attrs_eq_p): Declare.

	* gcc.dg/pr62004.c: New test.
	* gcc.dg/pr62030.c: Same.
	* gcc.target/mips/pr62030-octeon.c: Same.

diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index b5278bf..1cac518 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -263,7 +263,7 @@ mem_attrs_htab_hash (const void *x)
 
 /* Return true if the given memory attributes are equal.  */
 
-static bool
+bool
 mem_attrs_eq_p (const struct mem_attrs *p, const struct mem_attrs *q)
 {
   return (p->alias == q->alias
diff --git a/gcc/emit-rtl.h b/gcc/emit-rtl.h
index 7268090..9ccee07 100644
--- a/gcc/emit-rtl.h
+++ b/gcc/emit-rtl.h
@@ -69,6 +69,7 @@ extern void set_reg_attrs_for_parm (rtx, rtx);
 extern void set_reg_attrs_for_decl_rtl (tree t, rtx x);
 extern void adjust_reg_mode (rtx, enum machine_mode);
 extern int mem_expr_equal_p (const_tree, const_tree);
+extern bool mem_attrs_eq_p (const struct mem_attrs *, const struct mem_attrs *);
 
 extern bool need_atomic_barrier_p (enum memmodel, bool);
 
diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
index e3353a5..dd6df5b 100644
--- a/gcc/ifcvt.c
+++ b/gcc/ifcvt.c
@@ -294,6 +294,28 @@ block_fallthru (basic_block bb)
 
   return (e) ? e->dest : NULL_BLOCK;
 }
+
+/* Return true if RTXs A and B can be safely interchanged.  */
+
+static bool
+rtx_interchangeable_p (const_rtx a, const_rtx b)
+{
+  if (!rtx_equal_p (a, b))
+    return false;
+
+  if (GET_CODE (a) != MEM)
+    return true;
+
+  /* A dead type-unsafe memory reference is legal, but a live type-unsafe memory
+     reference is not.  Interchanging a dead type-unsafe memory reference with
+     a live type-safe one creates a live type-unsafe memory reference, in other
+     words, it makes the program illegal.
+     We check here conservatively whether the two memory references have equal
+     memory attributes.  */
+
+  return mem_attrs_eq_p (get_mem_attrs (a), get_mem_attrs (b));
+}
+
 \f
 /* Go through a bunch of insns, converting them to conditional
    execution format if possible.  Return TRUE if all of the non-note
@@ -1014,6 +1036,9 @@ noce_try_move (struct noce_if_info *if_info)
       || (rtx_equal_p (if_info->a, XEXP (cond, 1))
 	  && rtx_equal_p (if_info->b, XEXP (cond, 0))))
     {
+      if (!rtx_interchangeable_p (if_info->a, if_info->b))
+	return FALSE;
+
       y = (code == EQ) ? if_info->a : if_info->b;
 
       /* Avoid generating the move if the source is the destination.  */
@@ -2483,7 +2508,7 @@ noce_process_if_block (struct noce_if_info *if_info)
       if (! insn_b
 	  || insn_b != last_active_insn (else_bb, FALSE)
 	  || (set_b = single_set (insn_b)) == NULL_RTX
-	  || ! rtx_equal_p (x, SET_DEST (set_b)))
+	  || ! rtx_interchangeable_p (x, SET_DEST (set_b)))
 	return FALSE;
     }
   else
@@ -2496,7 +2521,7 @@ noce_process_if_block (struct noce_if_info *if_info)
 	  || BLOCK_FOR_INSN (insn_b) != BLOCK_FOR_INSN (if_info->cond_earliest)
 	  || !NONJUMP_INSN_P (insn_b)
 	  || (set_b = single_set (insn_b)) == NULL_RTX
-	  || ! rtx_equal_p (x, SET_DEST (set_b))
+	  || ! rtx_interchangeable_p (x, SET_DEST (set_b))
 	  || ! noce_operand_ok (SET_SRC (set_b))
 	  || reg_overlap_mentioned_p (x, SET_SRC (set_b))
 	  || modified_between_p (SET_SRC (set_b), insn_b, jump)
@@ -2562,7 +2587,7 @@ noce_process_if_block (struct noce_if_info *if_info)
 
   /* Look and see if A and B are really the same.  Avoid creating silly
      cmove constructs that no one will fix up later.  */
-  if (rtx_equal_p (a, b))
+  if (rtx_interchangeable_p (a, b))
     {
       /* If we have an INSN_B, we don't have to create any new rtl.  Just
 	 move the instruction that we already have.  If we don't have an
diff --git a/gcc/testsuite/gcc.dg/pr62004.c b/gcc/testsuite/gcc.dg/pr62004.c
new file mode 100644
index 0000000..c994a41
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr62004.c
@@ -0,0 +1,47 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -fno-tree-tail-merge" } */
+
+struct node
+{
+  struct node *next;
+  struct node *prev;
+};
+
+struct node node;
+
+struct head
+{
+  struct node *first;
+};
+
+struct head heads[5];
+
+int k = 2;
+
+struct head *head = &heads[2];
+
+int
+main ()
+{
+  struct node *p;
+
+  node.next = (void*)0;
+
+  node.prev = (void *)head;
+
+  head->first = &node;
+
+  struct node *n = head->first;
+
+  struct head *h = &heads[k];
+
+  heads[2].first = n->next;
+
+  if ((void*)n->prev == (void *)h)
+    p = h->first;
+  else
+    /* Dead tbaa-unsafe load from ((struct node *)&heads[2])->next.  */
+    p = n->prev->next;
+
+  return !(p == (void*)0);
+}
diff --git a/gcc/testsuite/gcc.dg/pr62030.c b/gcc/testsuite/gcc.dg/pr62030.c
new file mode 100644
index 0000000..b8baf93
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr62030.c
@@ -0,0 +1,50 @@
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+extern void abort (void);
+
+struct node
+{
+  struct node *next;
+  struct node *prev;
+};
+
+struct node node;
+
+struct head
+{
+  struct node *first;
+};
+
+struct head heads[5];
+
+int k = 2;
+
+struct head *head = &heads[2];
+
+static int __attribute__((noinline))
+foo (void)
+{
+  node.prev = (void *)head;
+  head->first = &node;
+
+  struct node *n = head->first;
+  struct head *h = &heads[k];
+  struct node *next = n->next;
+
+  if (n->prev == (void *)h)
+    h->first = next;
+  else
+    n->prev->next = next;
+
+  n->next = h->first;
+  return n->next == &node;
+}
+
+int
+main (void)
+{
+  if (foo ())
+    abort ();
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/mips/pr62030-octeon.c b/gcc/testsuite/gcc.target/mips/pr62030-octeon.c
new file mode 100644
index 0000000..5e3d3b3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/pr62030-octeon.c
@@ -0,0 +1,50 @@
+/* { dg-do run } */
+/* { dg-options "-march=octeon" } */
+
+extern void abort (void);
+
+struct node
+{
+  struct node *next;
+  struct node *prev;
+};
+
+struct node node;
+
+struct head
+{
+  struct node *first;
+};
+
+struct head heads[5];
+
+int k = 2;
+
+struct head *head = &heads[2];
+
+static int __attribute__((noinline))
+foo (void)
+{
+  node.prev = (void *)head;
+  head->first = &node;
+
+  struct node *n = head->first;
+  struct head *h = &heads[k];
+  struct node *next = n->next;
+
+  if (n->prev == (void *)h)
+    h->first = next;
+  else
+    n->prev->next = next;
+
+  n->next = h->first;
+  return n->next == &node;
+}
+
+int
+main (void)
+{
+  if (foo ())
+    abort ();
+  return 0;
+}
-- 
1.9.1

Re: Fix if-conversion pass for dead type-unsafe code

[Tom de Vries]

[-- Attachment #1: Type: text/plain, Size: 422 bytes --]

On 08-08-14 17:17, Tom de Vries wrote:
>> Maybe instead of a new mem_alias_equal_p simply compare MEM_ATTRs
>> with mem_attrs_eq_p?
>
> I propose to fix it this way (as attached) on 4.8/4.9/trunk, and maybe do a more
> efficient handling on trunk as a follow-up patch.
>
> I'll put this through bootstrap/test again.

Bootstrapped and reg-tested on trunk x86_64.

Re-attached patch OK for trunk, 4.8, 4.9 ?

Thanks,
- Tom

[-- Attachment #2: 0001-Conservative-fix-for-PR62004-PR62030.patch --]
[-- Type: text/x-patch, Size: 6642 bytes --]

2014-08-08  Tom de Vries  <tom@codesourcery.com>

	PR rtl-optimization/62004
	PR rtl-optimization/62030
	* ifcvt.c (rtx_interchangeable_p): New function.
	(noce_try_move, noce_process_if_block): Use rtx_interchangeable_p.
	* emit-rtl.c (mem_attrs_eq_p): Remove static.
	* emit-rtl.h (mem_attrs_eq_p): Declare.

	* gcc.dg/pr62004.c: New test.
	* gcc.dg/pr62030.c: Same.
	* gcc.target/mips/pr62030-octeon.c: Same.

diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index b5278bf..1cac518 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -263,7 +263,7 @@ mem_attrs_htab_hash (const void *x)
 
 /* Return true if the given memory attributes are equal.  */
 
-static bool
+bool
 mem_attrs_eq_p (const struct mem_attrs *p, const struct mem_attrs *q)
 {
   return (p->alias == q->alias
diff --git a/gcc/emit-rtl.h b/gcc/emit-rtl.h
index 7268090..9ccee07 100644
--- a/gcc/emit-rtl.h
+++ b/gcc/emit-rtl.h
@@ -69,6 +69,7 @@ extern void set_reg_attrs_for_parm (rtx, rtx);
 extern void set_reg_attrs_for_decl_rtl (tree t, rtx x);
 extern void adjust_reg_mode (rtx, enum machine_mode);
 extern int mem_expr_equal_p (const_tree, const_tree);
+extern bool mem_attrs_eq_p (const struct mem_attrs *, const struct mem_attrs *);
 
 extern bool need_atomic_barrier_p (enum memmodel, bool);
 
diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
index e3353a5..dd6df5b 100644
--- a/gcc/ifcvt.c
+++ b/gcc/ifcvt.c
@@ -294,6 +294,28 @@ block_fallthru (basic_block bb)
 
   return (e) ? e->dest : NULL_BLOCK;
 }
+
+/* Return true if RTXs A and B can be safely interchanged.  */
+
+static bool
+rtx_interchangeable_p (const_rtx a, const_rtx b)
+{
+  if (!rtx_equal_p (a, b))
+    return false;
+
+  if (GET_CODE (a) != MEM)
+    return true;
+
+  /* A dead type-unsafe memory reference is legal, but a live type-unsafe memory
+     reference is not.  Interchanging a dead type-unsafe memory reference with
+     a live type-safe one creates a live type-unsafe memory reference, in other
+     words, it makes the program illegal.
+     We check here conservatively whether the two memory references have equal
+     memory attributes.  */
+
+  return mem_attrs_eq_p (get_mem_attrs (a), get_mem_attrs (b));
+}
+
 \f
 /* Go through a bunch of insns, converting them to conditional
    execution format if possible.  Return TRUE if all of the non-note
@@ -1014,6 +1036,9 @@ noce_try_move (struct noce_if_info *if_info)
       || (rtx_equal_p (if_info->a, XEXP (cond, 1))
 	  && rtx_equal_p (if_info->b, XEXP (cond, 0))))
     {
+      if (!rtx_interchangeable_p (if_info->a, if_info->b))
+	return FALSE;
+
       y = (code == EQ) ? if_info->a : if_info->b;
 
       /* Avoid generating the move if the source is the destination.  */
@@ -2483,7 +2508,7 @@ noce_process_if_block (struct noce_if_info *if_info)
       if (! insn_b
 	  || insn_b != last_active_insn (else_bb, FALSE)
 	  || (set_b = single_set (insn_b)) == NULL_RTX
-	  || ! rtx_equal_p (x, SET_DEST (set_b)))
+	  || ! rtx_interchangeable_p (x, SET_DEST (set_b)))
 	return FALSE;
     }
   else
@@ -2496,7 +2521,7 @@ noce_process_if_block (struct noce_if_info *if_info)
 	  || BLOCK_FOR_INSN (insn_b) != BLOCK_FOR_INSN (if_info->cond_earliest)
 	  || !NONJUMP_INSN_P (insn_b)
 	  || (set_b = single_set (insn_b)) == NULL_RTX
-	  || ! rtx_equal_p (x, SET_DEST (set_b))
+	  || ! rtx_interchangeable_p (x, SET_DEST (set_b))
 	  || ! noce_operand_ok (SET_SRC (set_b))
 	  || reg_overlap_mentioned_p (x, SET_SRC (set_b))
 	  || modified_between_p (SET_SRC (set_b), insn_b, jump)
@@ -2562,7 +2587,7 @@ noce_process_if_block (struct noce_if_info *if_info)
 
   /* Look and see if A and B are really the same.  Avoid creating silly
      cmove constructs that no one will fix up later.  */
-  if (rtx_equal_p (a, b))
+  if (rtx_interchangeable_p (a, b))
     {
       /* If we have an INSN_B, we don't have to create any new rtl.  Just
 	 move the instruction that we already have.  If we don't have an
diff --git a/gcc/testsuite/gcc.dg/pr62004.c b/gcc/testsuite/gcc.dg/pr62004.c
new file mode 100644
index 0000000..c994a41
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr62004.c
@@ -0,0 +1,47 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -fno-tree-tail-merge" } */
+
+struct node
+{
+  struct node *next;
+  struct node *prev;
+};
+
+struct node node;
+
+struct head
+{
+  struct node *first;
+};
+
+struct head heads[5];
+
+int k = 2;
+
+struct head *head = &heads[2];
+
+int
+main ()
+{
+  struct node *p;
+
+  node.next = (void*)0;
+
+  node.prev = (void *)head;
+
+  head->first = &node;
+
+  struct node *n = head->first;
+
+  struct head *h = &heads[k];
+
+  heads[2].first = n->next;
+
+  if ((void*)n->prev == (void *)h)
+    p = h->first;
+  else
+    /* Dead tbaa-unsafe load from ((struct node *)&heads[2])->next.  */
+    p = n->prev->next;
+
+  return !(p == (void*)0);
+}
diff --git a/gcc/testsuite/gcc.dg/pr62030.c b/gcc/testsuite/gcc.dg/pr62030.c
new file mode 100644
index 0000000..b8baf93
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr62030.c
@@ -0,0 +1,50 @@
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+extern void abort (void);
+
+struct node
+{
+  struct node *next;
+  struct node *prev;
+};
+
+struct node node;
+
+struct head
+{
+  struct node *first;
+};
+
+struct head heads[5];
+
+int k = 2;
+
+struct head *head = &heads[2];
+
+static int __attribute__((noinline))
+foo (void)
+{
+  node.prev = (void *)head;
+  head->first = &node;
+
+  struct node *n = head->first;
+  struct head *h = &heads[k];
+  struct node *next = n->next;
+
+  if (n->prev == (void *)h)
+    h->first = next;
+  else
+    n->prev->next = next;
+
+  n->next = h->first;
+  return n->next == &node;
+}
+
+int
+main (void)
+{
+  if (foo ())
+    abort ();
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/mips/pr62030-octeon.c b/gcc/testsuite/gcc.target/mips/pr62030-octeon.c
new file mode 100644
index 0000000..5e3d3b3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/pr62030-octeon.c
@@ -0,0 +1,50 @@
+/* { dg-do run } */
+/* { dg-options "-march=octeon" } */
+
+extern void abort (void);
+
+struct node
+{
+  struct node *next;
+  struct node *prev;
+};
+
+struct node node;
+
+struct head
+{
+  struct node *first;
+};
+
+struct head heads[5];
+
+int k = 2;
+
+struct head *head = &heads[2];
+
+static int __attribute__((noinline))
+foo (void)
+{
+  node.prev = (void *)head;
+  head->first = &node;
+
+  struct node *n = head->first;
+  struct head *h = &heads[k];
+  struct node *next = n->next;
+
+  if (n->prev == (void *)h)
+    h->first = next;
+  else
+    n->prev->next = next;
+
+  n->next = h->first;
+  return n->next == &node;
+}
+
+int
+main (void)
+{
+  if (foo ())
+    abort ();
+  return 0;
+}
-- 
1.9.1

Re: Fix if-conversion pass for dead type-unsafe code

[Richard Biener]

On Sat, Aug 9, 2014 at 7:14 AM, Tom de Vries <Tom_deVries@mentor.com> wrote:
> On 08-08-14 17:17, Tom de Vries wrote:
>>>
>>> Maybe instead of a new mem_alias_equal_p simply compare MEM_ATTRs
>>> with mem_attrs_eq_p?
>>
>>
>> I propose to fix it this way (as attached) on 4.8/4.9/trunk, and maybe do
>> a more
>> efficient handling on trunk as a follow-up patch.
>>
>> I'll put this through bootstrap/test again.
>
>
> Bootstrapped and reg-tested on trunk x86_64.
>
> Re-attached patch OK for trunk, 4.8, 4.9 ?

Ok.  (did you check the effect on code-generation?  that is, how many
opportunities compiling GCC do we "lose"?)

Thanks,
Richard.

> Thanks,
> - Tom

Re: Fix if-conversion pass for dead type-unsafe code

[Tom de Vries]

[-- Attachment #1: Type: text/plain, Size: 1085 bytes --]

On 14-08-14 16:34, Richard Biener wrote:
> On Sat, Aug 9, 2014 at 7:14 AM, Tom de Vries <Tom_deVries@mentor.com> wrote:
>> On 08-08-14 17:17, Tom de Vries wrote:
>>>>
>>>> Maybe instead of a new mem_alias_equal_p simply compare MEM_ATTRs
>>>> with mem_attrs_eq_p?
>>>
>>>
>>> I propose to fix it this way (as attached) on 4.8/4.9/trunk, and maybe do
>>> a more
>>> efficient handling on trunk as a follow-up patch.
>>>
>>> I'll put this through bootstrap/test again.
>>
>>
>> Bootstrapped and reg-tested on trunk x86_64.
>>
>> Re-attached patch OK for trunk, 4.8, 4.9 ?
>
> Ok.

Backported to 4.8 and 4.9 as attached.

The emit-rtl.{c,h} part of my patch was superfluous on trunk given that you 
already exported mem_attrs_eq_p, something I failed to notice when rebasing. So 
the backport contains that part of your patch. I've tested the backport patch on 
4.9.

> (did you check the effect on code-generation?  that is, how many
> opportunities compiling GCC do we "lose"?)
>

I haven't done that. I can look into that (after I fix the tail-merge part of 
pr62004).

Thanks,
- Tom


[-- Attachment #2: 0001-Fix-if-conversion-pass-for-dead-type-unsafe-code.patch --]
[-- Type: text/x-patch, Size: 6742 bytes --]

2014-08-15  Tom de Vries  <tom@codesourcery.com>

	Backport from mainline:
	2014-08-14  Tom de Vries  <tom@codesourcery.com>

	PR rtl-optimization/62004
	PR rtl-optimization/62030
	* ifcvt.c (rtx_interchangeable_p): New function.
	(noce_try_move, noce_process_if_block): Use rtx_interchangeable_p.

	* gcc.dg/pr62004.c: New test.
	* gcc.dg/pr62030.c: Same.
	* gcc.target/mips/pr62030-octeon.c: Same.

	2014-08-05  Richard Biener  <rguenther@suse.de>

	* emit-rtl.h (mem_attrs_eq_p): Declare.
	* emit-rtl.c (mem_attrs_eq_p): Export.

diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index 4736f8d..89b6768 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -245,7 +245,7 @@ const_fixed_htab_eq (const void *x, const void *y)
 
 /* Return true if the given memory attributes are equal.  */
 
-static bool
+bool
 mem_attrs_eq_p (const struct mem_attrs *p, const struct mem_attrs *q)
 {
   return (p->alias == q->alias
diff --git a/gcc/emit-rtl.h b/gcc/emit-rtl.h
index fe68de9..36eb0c8 100644
--- a/gcc/emit-rtl.h
+++ b/gcc/emit-rtl.h
@@ -20,6 +20,9 @@ along with GCC; see the file COPYING3.  If not see
 #ifndef GCC_EMIT_RTL_H
 #define GCC_EMIT_RTL_H
 
+/* Return whether two MEM_ATTRs are equal.  */
+bool mem_attrs_eq_p (const struct mem_attrs *, const struct mem_attrs *);
+
 /* Set the alias set of MEM to SET.  */
 extern void set_mem_alias_set (rtx, alias_set_type);
 
diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
index 0d1adce..49ff85c 100644
--- a/gcc/ifcvt.c
+++ b/gcc/ifcvt.c
@@ -306,6 +306,28 @@ block_fallthru (basic_block bb)
 
   return (e) ? e->dest : NULL_BLOCK;
 }
+
+/* Return true if RTXs A and B can be safely interchanged.  */
+
+static bool
+rtx_interchangeable_p (const_rtx a, const_rtx b)
+{
+  if (!rtx_equal_p (a, b))
+    return false;
+
+  if (GET_CODE (a) != MEM)
+    return true;
+
+  /* A dead type-unsafe memory reference is legal, but a live type-unsafe memory
+     reference is not.  Interchanging a dead type-unsafe memory reference with
+     a live type-safe one creates a live type-unsafe memory reference, in other
+     words, it makes the program illegal.
+     We check here conservatively whether the two memory references have equal
+     memory attributes.  */
+
+  return mem_attrs_eq_p (get_mem_attrs (a), get_mem_attrs (b));
+}
+
 \f
 /* Go through a bunch of insns, converting them to conditional
    execution format if possible.  Return TRUE if all of the non-note
@@ -1034,6 +1056,9 @@ noce_try_move (struct noce_if_info *if_info)
       || (rtx_equal_p (if_info->a, XEXP (cond, 1))
 	  && rtx_equal_p (if_info->b, XEXP (cond, 0))))
     {
+      if (!rtx_interchangeable_p (if_info->a, if_info->b))
+	return FALSE;
+
       y = (code == EQ) ? if_info->a : if_info->b;
 
       /* Avoid generating the move if the source is the destination.  */
@@ -2504,7 +2529,7 @@ noce_process_if_block (struct noce_if_info *if_info)
       if (! insn_b
 	  || insn_b != last_active_insn (else_bb, FALSE)
 	  || (set_b = single_set (insn_b)) == NULL_RTX
-	  || ! rtx_equal_p (x, SET_DEST (set_b)))
+	  || ! rtx_interchangeable_p (x, SET_DEST (set_b)))
 	return FALSE;
     }
   else
@@ -2517,7 +2542,7 @@ noce_process_if_block (struct noce_if_info *if_info)
 	  || BLOCK_FOR_INSN (insn_b) != BLOCK_FOR_INSN (if_info->cond_earliest)
 	  || !NONJUMP_INSN_P (insn_b)
 	  || (set_b = single_set (insn_b)) == NULL_RTX
-	  || ! rtx_equal_p (x, SET_DEST (set_b))
+	  || ! rtx_interchangeable_p (x, SET_DEST (set_b))
 	  || ! noce_operand_ok (SET_SRC (set_b))
 	  || reg_overlap_mentioned_p (x, SET_SRC (set_b))
 	  || modified_between_p (SET_SRC (set_b), insn_b, jump)
@@ -2583,7 +2608,7 @@ noce_process_if_block (struct noce_if_info *if_info)
 
   /* Look and see if A and B are really the same.  Avoid creating silly
      cmove constructs that no one will fix up later.  */
-  if (rtx_equal_p (a, b))
+  if (rtx_interchangeable_p (a, b))
     {
       /* If we have an INSN_B, we don't have to create any new rtl.  Just
 	 move the instruction that we already have.  If we don't have an
diff --git a/gcc/testsuite/gcc.dg/pr62004.c b/gcc/testsuite/gcc.dg/pr62004.c
new file mode 100644
index 0000000..c994a41
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr62004.c
@@ -0,0 +1,47 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -fno-tree-tail-merge" } */
+
+struct node
+{
+  struct node *next;
+  struct node *prev;
+};
+
+struct node node;
+
+struct head
+{
+  struct node *first;
+};
+
+struct head heads[5];
+
+int k = 2;
+
+struct head *head = &heads[2];
+
+int
+main ()
+{
+  struct node *p;
+
+  node.next = (void*)0;
+
+  node.prev = (void *)head;
+
+  head->first = &node;
+
+  struct node *n = head->first;
+
+  struct head *h = &heads[k];
+
+  heads[2].first = n->next;
+
+  if ((void*)n->prev == (void *)h)
+    p = h->first;
+  else
+    /* Dead tbaa-unsafe load from ((struct node *)&heads[2])->next.  */
+    p = n->prev->next;
+
+  return !(p == (void*)0);
+}
diff --git a/gcc/testsuite/gcc.dg/pr62030.c b/gcc/testsuite/gcc.dg/pr62030.c
new file mode 100644
index 0000000..b8baf93
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr62030.c
@@ -0,0 +1,50 @@
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+extern void abort (void);
+
+struct node
+{
+  struct node *next;
+  struct node *prev;
+};
+
+struct node node;
+
+struct head
+{
+  struct node *first;
+};
+
+struct head heads[5];
+
+int k = 2;
+
+struct head *head = &heads[2];
+
+static int __attribute__((noinline))
+foo (void)
+{
+  node.prev = (void *)head;
+  head->first = &node;
+
+  struct node *n = head->first;
+  struct head *h = &heads[k];
+  struct node *next = n->next;
+
+  if (n->prev == (void *)h)
+    h->first = next;
+  else
+    n->prev->next = next;
+
+  n->next = h->first;
+  return n->next == &node;
+}
+
+int
+main (void)
+{
+  if (foo ())
+    abort ();
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/mips/pr62030-octeon.c b/gcc/testsuite/gcc.target/mips/pr62030-octeon.c
new file mode 100644
index 0000000..5e3d3b3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/pr62030-octeon.c
@@ -0,0 +1,50 @@
+/* { dg-do run } */
+/* { dg-options "-march=octeon" } */
+
+extern void abort (void);
+
+struct node
+{
+  struct node *next;
+  struct node *prev;
+};
+
+struct node node;
+
+struct head
+{
+  struct node *first;
+};
+
+struct head heads[5];
+
+int k = 2;
+
+struct head *head = &heads[2];
+
+static int __attribute__((noinline))
+foo (void)
+{
+  node.prev = (void *)head;
+  head->first = &node;
+
+  struct node *n = head->first;
+  struct head *h = &heads[k];
+  struct node *next = n->next;
+
+  if (n->prev == (void *)h)
+    h->first = next;
+  else
+    n->prev->next = next;
+
+  n->next = h->first;
+  return n->next == &node;
+}
+
+int
+main (void)
+{
+  if (foo ())
+    abort ();
+  return 0;
+}
-- 
1.9.1

Re: Fix if-conversion pass for dead type-unsafe code

[pinskia]

> On Aug 18, 2014, at 1:33 AM, Tom de Vries <Tom_deVries@mentor.com> wrote:
> 
>> On 14-08-14 16:34, Richard Biener wrote:
>>> On Sat, Aug 9, 2014 at 7:14 AM, Tom de Vries <Tom_deVries@mentor.com> wrote:
>>> On 08-08-14 17:17, Tom de Vries wrote:
>>>>> 
>>>>> Maybe instead of a new mem_alias_equal_p simply compare MEM_ATTRs
>>>>> with mem_attrs_eq_p?
>>>> 
>>>> 
>>>> I propose to fix it this way (as attached) on 4.8/4.9/trunk, and maybe do
>>>> a more
>>>> efficient handling on trunk as a follow-up patch.
>>>> 
>>>> I'll put this through bootstrap/test again.
>>> 
>>> 
>>> Bootstrapped and reg-tested on trunk x86_64.
>>> 
>>> Re-attached patch OK for trunk, 4.8, 4.9 ?
>> 
>> Ok.
> 
> Backported to 4.8 and 4.9 as attached.
> 
> The emit-rtl.{c,h} part of my patch was superfluous on trunk given that you already exported mem_attrs_eq_p, something I failed to notice when rebasing. So the backport contains that part of your patch. I've tested the backport patch on 4.9.
> 
>> (did you check the effect on code-generation?  that is, how many
>> opportunities compiling GCC do we "lose"?)
> 
> I haven't done that. I can look into that (after I fix the tail-merge part of pr62004).

Just FYI, this did fix the issue on MIPS that I had saw. 

Thanks,
Andrew

> 
> Thanks,
> - Tom
> 
> <0001-Fix-if-conversion-pass-for-dead-type-unsafe-code.patch>