[Bug tree-optimization/107346] New: gnat.dg/loop_optimization23_pkg.ad failure afer r13-3413-ge10ca9544632db

[Bug tree-optimization/107346] New: gnat.dg/loop_optimization23_pkg.ad failure afer r13-3413-ge10ca9544632db

[avieira at gcc dot gnu.org]

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

            Bug ID: 107346
           Summary: gnat.dg/loop_optimization23_pkg.ad failure afer
                    r13-3413-ge10ca9544632db
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: avieira at gcc dot gnu.org
  Target Milestone: ---

As reported by Eric in
https://gcc.gnu.org/pipermail/gcc-patches/2022-October/603356.html

[Bug tree-optimization/107346] gnat.dg/loop_optimization23_pkg.ad failure afer r13-3413-ge10ca9544632db

[avieira at gcc dot gnu.org]

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

avieira at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |avieira at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-10-21

--- Comment #1 from avieira at gcc dot gnu.org ---
I've tracked this down to 'vect_check_gather_scatter's pbytepos calculation:

poly_int64 pbytepos = exact_div (pbitpos, BITS_PER_UNIT);

Where pbitpos is 4 and that triggers an assert in exact_div. I am not sure what
the best fix would be here. The stmt this fails on is:
_ifc__23 = (*x_7(D))[_1].b.D.3707;

But I am having trouble debugging this as I cant' seem to break on
vect_recog_bit_insert_pattern and I haven't figured out how to get gnat1 to
create dumps :(

[Bug tree-optimization/107346] gnat.dg/loop_optimization23_pkg.ad failure afer r13-3413-ge10ca9544632db

[rguenth at gcc dot gnu.org]

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

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
I think the issue was probably latent and the function assumes that bitfield
accesses are rejected earlier but now are no longer(?).  vect_analyze_data_refs
is usually responsible for that, eventually we also rely on data-ref analysis
itself to reject bitfield accesses.

[Bug tree-optimization/107346] [13 Regression] gnat.dg/loop_optimization23_pkg.ad failure afer r13-3413-ge10ca9544632db

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|gnat.dg/loop_optimization23 |[13 Regression]
                   |_pkg.ad failure afer        |gnat.dg/loop_optimization23
                   |r13-3413-ge10ca9544632db    |_pkg.ad failure afer
                   |                            |r13-3413-ge10ca9544632db
           Keywords|                            |ice-on-valid-code
           Priority|P3                          |P1
            Version|unknown                     |13.0
   Target Milestone|---                         |13.0

[Bug tree-optimization/107346] [13 Regression] gnat.dg/loop_optimization23_pkg.ad failure afer r13-3413-ge10ca9544632db

[avieira at gcc dot gnu.org]

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

--- Comment #3 from avieira at gcc dot gnu.org ---
I am wondering whether I should try to support this, or bail out of
vect_check_gather_scatter if pbitpos is not a multiple of BITS_PER_UNIT. The
latter obviously feels safer.

[Bug tree-optimization/107346] [13 Regression] gnat.dg/loop_optimization23_pkg.ad failure afer r13-3413-ge10ca9544632db

[avieira at gcc dot gnu.org]

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

--- Comment #4 from avieira at gcc dot gnu.org ---
Funnily enough, if I transform the Int24 into a 32-bit integer in the testcase
and disable all bitfield lowering just to make sure, I get the same failure. I
tried using __attribute__((packed)) in C to reproduce this, but I keep getting
a 32-bit offset... Either way, I will test a patch where
vect_check_gather_scatter bails out if pbitpos isn't a multiple of
BITS_PER_UNIT.

[Bug tree-optimization/107346] [13 Regression] gnat.dg/loop_optimization23_pkg.ad failure afer r13-3413-ge10ca9544632db

[ebotcazou at gcc dot gnu.org]

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

--- Comment #5 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> But I am having trouble debugging this as I cant' seem to break on
> vect_recog_bit_insert_pattern and I haven't figured out how to get gnat1 to
> create dumps :(

There are no differences between gnat1 and cc1/cc1plus as far as dumps are
concerned, e.g. -fdump-tree-optimized creates the .optimized dump.

[Bug tree-optimization/107346] [13 Regression] gnat.dg/loop_optimization23_pkg.ad failure afer r13-3413-ge10ca9544632db

[avieira at gcc dot gnu.org]

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

--- Comment #6 from avieira at gcc dot gnu.org ---
> There are no differences between gnat1 and cc1/cc1plus as far as dumps are concerned, e.g. -fdump-tree-optimized creates the .optimized dump.

This was my bad, I'm not used to using cc1 directly, usually go through the
driver, so didn't realize it was putting the dumps in the same place as the
source file.

[Bug tree-optimization/107346] [13 Regression] gnat.dg/loop_optimization23_pkg.ad failure afer r13-3413-ge10ca9544632db

[ebotcazou at gcc dot gnu.org]

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

--- Comment #7 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> I am wondering whether I should try to support this, or bail out of
> vect_check_gather_scatter if pbitpos is not a multiple of BITS_PER_UNIT. The
> latter obviously feels safer.

Probably the way to go indeed.

[Bug tree-optimization/107346] [13 Regression] gnat.dg/loop_optimization23_pkg.ad failure afer r13-3413-ge10ca9544632db

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

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

--- Comment #8 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> I am wondering whether I should try to support this, or bail out of
> vect_check_gather_scatter if pbitpos is not a multiple of BITS_PER_UNIT. The
> latter obviously feels safer.

It turns out that this vect_check_gather_scatter problem was latent since I can
reproduce it with the GCC 12 compiler in Ada, and the following fix:

diff --git a/gcc/tree-vect-data-refs.cc b/gcc/tree-vect-data-refs.cc
index e32b1779c5d..a22eb2d763d 100644
--- a/gcc/tree-vect-data-refs.cc
+++ b/gcc/tree-vect-data-refs.cc
@@ -4013,7 +4013,7 @@ vect_check_gather_scatter (stmt_vec_info stmt_info,
loop_vec_info loop_vinfo,
      that can be gimplified before the loop.  */
   base = get_inner_reference (base, &pbitsize, &pbitpos, &off, &pmode,
                              &punsignedp, &reversep, &pvolatilep);
-  if (reversep)
+  if (!multiple_p (pbitpos, BITS_PER_UNIT) || reversep)
     return false;

   poly_int64 pbytepos = exact_div (pbitpos, BITS_PER_UNIT);

is sufficient to get rid of it.

[Bug tree-optimization/107346] [13 Regression] gnat.dg/loop_optimization23_pkg.ad failure afer r13-3413-ge10ca9544632db

[avieira at gcc dot gnu.org]

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

--- Comment #9 from avieira at gcc dot gnu.org ---
Hi Eric,

I realised the same, got a patch pending here:
https://gcc.gnu.org/pipermail/gcc-patches/2022-October/604139.html

[Bug tree-optimization/107346] [13 Regression] gnat.dg/loop_optimization23_pkg.ad failure afer r13-3413-ge10ca9544632db

[cvs-commit at gcc dot gnu.org]

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

--- Comment #10 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:95decac3ce8c8c7c5302cd6fac005a10463de165

commit r13-3547-g95decac3ce8c8c7c5302cd6fac005a10463de165
Author: Andre Vieira <andre.simoesdiasvieira@arm.com>
Date:   Fri Oct 28 15:05:11 2022 +0100

    vect: Reject non-byte offsets for gather/scatters [PR107346]

    The ada failure reported in the PR was being caused by
vect_check_gather_scatter
    failing to deal with bit offsets that weren't multiples of BITS_PER_UNIT.
This
    patch makes vect_check_gather_scatter reject memory accesses with such
offsets.

    gcc/ChangeLog:

            PR tree-optimization/107346
            * tree-vect-data-refs.cc (vect_check_gather_scatter): Reject
offsets
            that aren't multiples of BITS_PER_UNIT.

[Bug tree-optimization/107346] [13 Regression] gnat.dg/loop_optimization23_pkg.ad failure afer r13-3413-ge10ca9544632db

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

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

--- Comment #11 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
Thanks for fixing the problem.