Re: gcse/cse: Revert parts of the PR25130 patch

[Richard Guenther <richard dot guenther at gmail dot com>]

On Mon, Jun 28, 2010 at 11:55 PM, Bernd Schmidt <bernds@codesourcery.com> wrote:
> I ran into some interesting problems when I was trying to change
> thumb1_legitimate_address. ?A minor change, which left initial RTL
> generation identical at first glance, produced optimization regressions
> later on in gcse.
>
> It turns out there are two paths for expanding things like ARRAY_REF or
> COMPONENT_REF, which are taken depending on whether direct_load is true
> for a given mode. ?In one of these paths, we call set_mem_attrs on the
> MEM we generate, in the other, we do not. ?(There'll probably need to be
> one or two bug fixes here later.)
>
> This causes gcse.c to behave differently for some testcases. ?When we
> don't set_mem_attrs, several MEMs are identical, and we get a single
> expression of the form
>
> (zero_extend:SI (mem/s:HI (plus:SI (reg/v/f:SI 149 [ o ])
> ? ? ? ? ? ? ? ?(const_int 16 [0x10])) [0+16 S2 A32])
>
> while in the other case, there's more information and we have
> different-looking MEMs:
>
> (zero_extend:SI (mem/s:HI (plus:SI (reg/v/f:SI 149 [ o ])
> ? ? ? ? ? ? ? ?(const_int 16 [0x10])) [6 o_3(D)->op_type+0 S2 A32]
>
> (zero_extend:SI (mem/s:HI (plus:SI (reg/v/f:SI 149 [ o ])
> ? ? ? ? ? ? ? ?(const_int 16 [0x10])) [6 o_1->op_type+0 S2 A32])
>
> The MEMs were generated as component_refs with different SSA names as
> pointers. ?They are identical in structure, but differ in MEM_ATTRS. ?In
> 2006, as a fix for PR25130, there was a change in how gcse would treat
> MEMs; previously, it only verified that the alias sets were the same,
> now it is very strict and tests MEM_ATTRS for equality, so it will no
> longer optimize the two latter expressions.
>
> This change seems to be unnecessary; I believe the real problem was
> fixed in PR41033. ?There, nonoverlapping_component_ref was disabled for
> the !flag_strict_aliasing case. ?In comment #9 for PR25130, we get a
> hint that RTL alias analysis is in fact the problem:
> canon_true_dependence returns false for two MEMs with identical
> structure (but different MEM_ATTRS).
>
> I verified with a 4.1 compiler that when the PR25130 patch is reverted
> and the one for PR41033 is applied, the compiler still produces correct
> output for the testcase. ?Based on this, I suggest the following patch,
> which reverts the important part of the PR25130 patch on mainline and
> fixes the optimization regressions I observed.
>
> Bootstrapped and regression tested (with some other changes) on
> i686-linux. ?Ok?

Ok.

Thanks,
Richard.

>
> Bernd
>