[Bug rtl-optimization/104498] New: Alias attribute being ignored by scheduler

[Bug rtl-optimization/104498] New: Alias attribute being ignored by scheduler

[avieira at gcc dot gnu.org]

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

            Bug ID: 104498
           Summary: Alias attribute being ignored by scheduler
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: avieira at gcc dot gnu.org
  Target Milestone: ---

Whilst working on a tuning structure I saw a correctness regression that I
believe is a result of the alias attribute not working properly.

You can reproduce it using an existing tuning for AArch64 using:
gcc -O2 src/gcc/gcc/testsuite/gcc.c-torture/execute/alias-2.c -S
-mtune=cortex-a34

This will lead to the 'a[off] = 2' store being moved after the b load in
'b[off] != 2'.

In RTL:

(insn 23 18 19 2 (set (reg:SI 110 [ b[off.0_1] ])
        (mem:SI (plus:DI (mult:DI (reg:DI 99 [ off.0_1 ])
                    (const_int 4 [0x4]))
                (reg/f:DI 97)) [1 b[off.0_1]+0 S4 A32]))
"gcc/gcc/testsuite/gcc.c-torture/execute/alias-2.c":10:6 52 {*movsi_aarch64}
     (expr_list:REG_DEAD (reg:DI 99 [ off.0_1 ])
        (expr_list:REG_DEAD (reg/f:DI 97)
            (nil))))
(insn 19 23 24 2 (set (mem:SI (plus:DI (mult:DI (reg:DI 99 [ off.0_1 ])
                    (const_int 4 [0x4]))
                (reg/f:DI 104)) [1 a[off.0_1]+0 S4 A32])
        (reg:SI 106)) "gcc/gcc/testsuite/gcc.c-torture/execute/alias-2.c":9:9
52 {*movsi_aarch64}
     (expr_list:REG_DEAD (reg:SI 106)
        (expr_list:REG_DEAD (reg/f:DI 104)
            (nil))))

After some debugging I found that true_dependence returns false for these two
memory accesses because base_alias_check sees they have different base objects
('a' and 'b') and deduces they can't alias based on that, without realising 'b'
isn't an actual base object but an alias to 'a'. I think we should make it so
that at expand pointers to 'b' get 'a' as a base object.

[Bug rtl-optimization/104498] Alias attribute being ignored by scheduler

[avieira at gcc dot gnu.org]

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

--- Comment #1 from avieira at gcc dot gnu.org ---
Forgot to mention, this happens during the sched1 pass.

[Bug rtl-optimization/104498] Alias attribute being ignored by scheduler

[rguenth at gcc dot gnu.org]

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

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
I don't see how it can with those MEMs.  Can you show the canonicalized (CSELIB
expanded) RTXen the base_alias_check routine sees?

[Bug rtl-optimization/104498] Alias attribute being ignored by scheduler

[avieira at gcc dot gnu.org]

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

--- Comment #3 from avieira at gcc dot gnu.org ---
Sorry some confusion there, I thought it was base_alias_check bailing out
early, but that seems to return true, it is the memrefs_conflict_p that returns
0.

I suspect rtx_equal_for_memref_p should have returned 1 for:
x:
(plus:DI (mult:DI (reg:DI 99 [ off.0_1 ])
        (const_int 4 [0x4]))
    (const:DI (plus:DI (symbol_ref:DI ("*.LANCHOR0") [flags 0x182])
            (const_int 16 [0x10]))))

and y:
(plus:DI (mult:DI (reg:DI 99 [ off.0_1 ])
        (const_int 4 [0x4]))
    (symbol_ref:DI ("b") [flags 0x2] <var_decl 0xfffff7635f30 b>))

But it does not... must be because of that trailing (equivalence notes? that's
what I assume they are?)

[Bug rtl-optimization/104498] Alias attribute being ignored by scheduler

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rsandifo at gcc dot gnu.org

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to avieira from comment #3)
> Sorry some confusion there, I thought it was base_alias_check bailing out
> early, but that seems to return true, it is the memrefs_conflict_p that
> returns 0.
> 
> I suspect rtx_equal_for_memref_p should have returned 1 for:
> x:
> (plus:DI (mult:DI (reg:DI 99 [ off.0_1 ])
>         (const_int 4 [0x4]))
>     (const:DI (plus:DI (symbol_ref:DI ("*.LANCHOR0") [flags 0x182])
>             (const_int 16 [0x10]))))
> 
> and y:
> (plus:DI (mult:DI (reg:DI 99 [ off.0_1 ])
>         (const_int 4 [0x4]))
>     (symbol_ref:DI ("b") [flags 0x2] <var_decl 0xfffff7635f30 b>))
> 
> But it does not... must be because of that trailing (equivalence notes?
> that's what I assume they are?)

It's the section anchor that breaks things.  There's a duplicate bug about this
somewhere, or a thread on the mailing list.  But IIRC Richard had fixed this?

[Bug rtl-optimization/104498] Alias attribute being ignored by scheduler

[avieira at gcc dot gnu.org]

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

--- Comment #5 from avieira at gcc dot gnu.org ---
You mean this https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92294

it only works for direct symbols I think it never enters
the block under: if (GET_CODE (x) == SYMBOL_REF && GET_CODE (y) == SYMBOL_REF)

which is where he made his changes. I'll go try to understand his changes
better, just had a quick look over.

[Bug rtl-optimization/104498] Alias attribute being ignored by scheduler

[rsandifo at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-02-11
             Status|UNCONFIRMED                 |NEW

--- Comment #6 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
The problem is that compare_base_symbol_refs can swap x and y:

      if (!x_decl)
        {
          std::swap (x_decl, y_decl);
          std::swap (x_base, y_base);
        }

but this doesn't change the polarity of the distance added here:

      if (distance)
        *distance += (SYMBOL_REF_BLOCK_OFFSET (y_base)
                      - SYMBOL_REF_BLOCK_OFFSET (x_base));

[Bug rtl-optimization/104498] Alias attribute being ignored by scheduler

[avieira at gcc dot gnu.org]

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

--- Comment #7 from avieira at gcc dot gnu.org ---
And I was thinking it didn't know how to handle anchor + offset...

Anyway if I just record the swap and use it to invert the distance calculation
that seems to 'work' for the testcase. I'm happy to go bootstrap it, or would
you rather fix this some other way?

[Bug rtl-optimization/104498] Alias attribute being ignored by scheduler

[rsandifo at gcc dot gnu.org]

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

--- Comment #8 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
No, negating the offset seems like the right fix.

[Bug rtl-optimization/104498] Alias attribute being ignored by scheduler

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Andre Simoes Dias Vieira
<avieira@gcc.gnu.org>:

https://gcc.gnu.org/g:d34cdec56728ddbdfacabd9f80598b17d1c6ff54

commit r12-7310-gd34cdec56728ddbdfacabd9f80598b17d1c6ff54
Author: Andre Vieira <andre.simoesdiasvieira@arm.com>
Date:   Mon Feb 21 09:41:53 2022 +0000

    rtl-optimization/104498: Fix comparing symbol reference

    gcc/ChangeLog:

            PR rtl-optimization/104498
            * alias.cc (compare_base_symbol_refs): Correct distance computation
            when swapping x and y.

[Bug rtl-optimization/104498] Alias attribute being ignored by scheduler

[avieira at gcc dot gnu.org]

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

avieira at gcc dot gnu.org changed:

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

--- Comment #10 from avieira at gcc dot gnu.org ---
Should be fixed with latest patch.