[Bug rtl-optimization/92294] alias attribute generates incorrect code

[Bug rtl-optimization/92294] alias attribute generates incorrect code

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92294

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.0                        |10.2

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 10.1 has been released.

[Bug rtl-optimization/92294] alias attribute generates incorrect code

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92294

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.2                        |10.3

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 10.2 is released, adjusting target milestone.

[Bug rtl-optimization/92294] alias attribute generates incorrect code

[mhillen at linux dot ibm.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92294

--- Comment #6 from Marius Hillenbrand <mhillen at linux dot ibm.com> ---
Same behavior on s390x: the testcase always calls abort(). As on aarch64,
-fno-section-anchors avoids the issue.

the first cse pass already makes a mistake -- on both aarch64 and s390x, the
compare looses its memory-read for the current value of b and instead reuses
the constant 1 assigned to b earlier.

on aarch64 that looks like

(insn # # # 2 (set (reg:SI 110)
        (mem:SI (plus:DI (mult:DI (reg:DI 109)
                    (const_int 4 [0x4]))
                (reg/f:DI 107)) [1 b[off.0_1]+0 S4 A32])) "alias-2.c":9:6#
{*movsi_aarch64}
     (nil))
(insn # # # 2 (set (reg:CC 66 cc)
        (compare:CC (reg:SI 110)
            (const_int 2 [0x2]))) "alias-2.c":9:6# {cmpsi}
     (nil))

... becomes ...

(insn # # # 2 (set (reg:SI 100)
        (const_int 1 [0x1])) "alias-2.c":7:9# {*movsi_aarch64}
     (nil)) [from the write to b]
...
(insn # # # 2 (set (reg:CC 66 cc)
        (compare:CC (reg:SI 100)
            (const_int 2 [0x2]))) "alias-2.c":9:6# {cmpsi}
     (expr_list:REG_DEAD (reg:SI 110 [ b[off.0_1] ])
        (nil)))

[Bug rtl-optimization/92294] alias attribute generates incorrect code

[mhillen at linux dot ibm.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92294

--- Comment #7 from Marius Hillenbrand <mhillen at linux dot ibm.com> ---
A simpler example derived from alias-2.c reproduces this issue on aarch64,
ppc64, and s390x. 

int a;
extern int b __attribute__ ((alias("a")));
int off;

int foo()
{
  /* make sure off is ahead of a and b in .bss, so that a has a non-negative
   * offset relative to the anchor. */
  return off;
}

main()
{
  b=1;
  a=2;
  if (b!=2)
   __builtin_abort ();
  return 0;
}

Note that gcc.c-torture/execute/alias-2.c did not reproduce this issue on
ppc64(le) for me. Removing the array indexing reduces the complexity of the RTL
dumps somewhat.

[Bug rtl-optimization/92294] alias attribute generates incorrect code

[mhillen at linux dot ibm.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92294

--- Comment #8 from Marius Hillenbrand <mhillen at linux dot ibm.com> ---
>From my current understanding, gcc addresses a and b in two different ways,
which is not handled correctly by the dependency analysis / alias analysis
while employed by the cse1 pass, and then causes cse1 to incorrectly eliminate
the read from b (reusing the const 1 from b=1).

the rtl addresses a via the section anchor yet b via its symbol reference:

b = 1 becomes
(set (reg/f:DI 62) (symbol_ref:DI ("b") [flags 0x602]  <var_decl .. b>))
(set (mem/c:SI (reg/f:DI 62) [1 b+0 S4 A32]) (const_int 1 [0x1]))

vs

a=2 becomes
(set (reg/f:DI 64) (symbol_ref:DI ("*.LANCHOR0") [flags 0x182]))
(set (mem/c:SI (plus:DI (reg/f:DI 64) (const_int 4 [0x4])) [1 a+0 S4 A32])
(const_int 2 [0x2]))

when visiting the write to a, cse1 should identify the aliasing and thus drop
the "remembered" b. for that, cse iterates its hash table and compares all
present MEM rtx to the new address (cse.c:invalidate()).

that step compares (canonicalized) addresses
(const:DI (plus:DI (symbol_ref:DI ("*.LANCHOR0") [flags 0x182]) (const_int 4
[0x4])))
and (symbol_ref:DI ("b") [flags 0x602]  <var_decl b>)

the comparison of these two (in alias.c:write_dependence_p()) falsely claims
that there is no anti-dependence between the two.

I am not certain yet how that comparison is supposed to work with section
anchors. It looks like we either fail to reconnect (anchor + 4) to the
symbol_ref of "a" (so we could identify both symbol_refs as equivalent) or lose
the offset relative to the anchor (so we could see that both have the same
offset relative to the anchor).

[Bug rtl-optimization/92294] alias attribute generates incorrect code

[rsandifo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92294

rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|                            |https://gcc.gnu.org/piperma
                   |                            |il/gcc-patches/2020-Februar
                   |                            |y/539763.html

--- Comment #9 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Oops, I never linked the patch, it's:
https://gcc.gnu.org/pipermail/gcc-patches/2020-February/539763.html
(following on from an earlier discussion in January).

Reading the thread back now, the reception wasn't as negative
as I'd remembered.  I'd been vaguely hoping to find time to try
removing find_base_term & co. for GCC 11, but that obviously
never happened.  (Turns out that writing new code is more fun. ;-))

Maybe we could consider going for the linked patch for GCC 11.

[Bug rtl-optimization/92294] alias attribute generates incorrect code

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92294

--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Sandiford <rsandifo@gcc.gnu.org>:

https://gcc.gnu.org/g:6a2a38620cf178b53b217051f32d1d7bbba86fc9

commit r11-6796-g6a2a38620cf178b53b217051f32d1d7bbba86fc9
Author: Richard Sandiford <richard.sandiford@arm.com>
Date:   Tue Jan 19 17:50:53 2021 +0000

    alias: Fix offset checks involving section anchors [PR92294]

    memrefs_conflict_p assumes that:

      [XB + XO, XB + XO + XS)

    does not alias

      [YB + YO, YB + YO + YS)

    whenever:

      [XO, XO + XS)

    does not intersect

      [YO, YO + YS)

    In other words, the accesses can alias only if XB == YB at runtime.

    However, this doesn't cope correctly with section anchors.
    For example, if XB is an anchor symbol and YB is at offset
    XO from the anchor, then:

      [XB + XO, XB + XO + XS)

    overlaps

      [YB, YB + YS)

    whatever the value of XO is.  In other words, when doing the
    alias check for two symbols whose local definitions are in
    the same block, we should apply the known difference between
    their block offsets to the intersection test above.

    gcc/
            PR rtl-optimization/92294
            * alias.c (compare_base_symbol_refs): Take an extra parameter
            and add the distance between two symbols to it.  Enshrine in
            comments that -1 means "either 0 or 1, but we can't tell
            which at compile time".
            (memrefs_conflict_p): Update call accordingly.
            (rtx_equal_for_memref_p): Likewise.  Take the distance between
symbols
            into account.

[Bug rtl-optimization/92294] alias attribute generates incorrect code

[rsandifo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92294

rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #11 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Fixed on trunk.  I'm not sure it's suitable for backports.

[Bug rtl-optimization/92294] alias attribute generates incorrect code

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92294

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.3                        |11.0