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.
