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

[Richard Biener <rguenther at suse dot de>]

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.