[Bug tree-optimization/111444] New: Wrong code at -O2/3/s on x86_64-gnu since r14-3226-gd073e2d75d9

[Bug tree-optimization/111444] New: Wrong code at -O2/3/s on x86_64-gnu since r14-3226-gd073e2d75d9

[shaohua.li at inf dot ethz.ch]

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

            Bug ID: 111444
           Summary: Wrong code at -O2/3/s on x86_64-gnu since
                    r14-3226-gd073e2d75d9
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: shaohua.li at inf dot ethz.ch
                CC: mjambor at suse dot cz
  Target Milestone: ---

gcc at -O2/3/s produced the wrong code.

Bisected to r14-3226-gd073e2d75d9

Compiler explorer: https://godbolt.org/z/xWj9z8foe

$ cat a.c
int printf(const char *, ...);
int a = 3, d, e;
int *b = &a;
char c;
short f;
const int **g;
static long h(int **i, int **j) {
  const int *k[46];
  const int **l = &k[5];
  *j = &e;
  g = l;
  for (; d; d = d + 1)
    ;
  **i = 0;
  return f;
}
int main() {
  int *m = &a;
  h(&m, &m);
  c = *b;
  printf("%d\n", c);
}
$
$ gcc -O0 a.c && ./a.out
3
$ gcc -O2 a.c && ./a.out
0
$

[Bug tree-optimization/111444] [14 Regression] Wrong code at -O2/3/s on x86_64-gnu since r14-3226-gd073e2d75d9

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
           Keywords|                            |alias, wrong-code
            Summary|Wrong code at -O2/3/s on    |[14 Regression] Wrong code
                   |x86_64-gnu since            |at -O2/3/s on x86_64-gnu
                   |r14-3226-gd073e2d75d9       |since r14-3226-gd073e2d75d9
   Last reconfirmed|                            |2023-09-17
     Ever confirmed|0                           |1
   Target Milestone|---                         |14.0

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Confirmed.

The code must not take into account that *i and *j could alias ???????

[Bug ipa/111444] [14 Regression] Wrong code at -O2/3/s on x86_64-gnu since r14-3226-gd073e2d75d9

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu.org
           Priority|P3                          |P1

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
Hmm, so we look for a *(int **) here, reach the toplevel vop, skipping *j = &e?
That should have conflicted.

[Bug ipa/111444] [14 Regression] Wrong code at -O2/3/s on x86_64-gnu since r14-3226-gd073e2d75d9

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Slightly reduced testcase:
int a = 3, d, e;
const int **g;

static void
foo (int **i, int **j)
{
  const int *k[46];
  const int **l = &k[5];
  *j = &e;
  for (g = l; d; d = d + 1)
    ;
  **i = 0;
}

int
main ()
{
  int *m = &a;
  foo (&m, &m);
  if (a != 3)
    __builtin_abort ();
}

This goes wrong during PRE.

[Bug ipa/111444] [14 Regression] Wrong code at -O2/3/s on x86_64-gnu since r14-3226-gd073e2d75d9

[jakub at gcc dot gnu.org]

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #2)
> Hmm, so we look for a *(int **) here, reach the toplevel vop, skipping *j =
> &e?
> That should have conflicted.

Seems we actually saw that.
walk_non_aliased_vuses calls walker (vn_reference_lookup_2) on .MEM_8 vuse,
that still returns NULL, then
3909              if (stmt_may_clobber_ref_p_1 (def_stmt, ref, tbaa_p))
3910                {
3911                  if (!translate)
3912                    break;
3913                  translate_flags disambiguate_only = TR_TRANSLATE;
3914                  res = (*translate) (ref, vuse, data, &disambiguate_only);
on the def_stmt:
# .MEM_8 = VDEF <.MEM_7(D)>
*j_1(D) = &e;
returns true from stmt_may_clobber_ref_p_1, correctly saying that *j_1(D) store
can clobber *i_1(D).
But then walk_non_aliased_vuses calls
3914                  res = (*translate) (ref, vuse, data, &disambiguate_only);
a few lines later, translate is vn_reference_lookup_3.
And vn_reference_lookup_3 calls vn_reference_lookup_2 here in:
              tree *saved_last_vuse_ptr = data->last_vuse_ptr;
              /* Do not update last_vuse_ptr in vn_reference_lookup_2.  */
              data->last_vuse_ptr = NULL;
              tree saved_vuse = vr->vuse;
              hashval_t saved_hashcode = vr->hashcode;
              void *res = vn_reference_lookup_2 (ref, gimple_vuse (def_stmt),
                                                 data);
              /* Need to restore vr->vuse and vr->hashcode.  */
              vr->vuse = saved_vuse;
              vr->hashcode = saved_hashcode;
              data->last_vuse_ptr = saved_last_vuse_ptr;
and def_stmt here is still the *j_1(D) = &e; statement, the problem is that
gimple_vuse (def_stmt) in that case is .MEM_7(D), so it triggers the r14-3226
if (SSA_NAME_IS_DEFAULT_DEF (vuse)) stuff at that point.  So, do we need to
somehow
arrange for the if (SSA_NAME_IS_DEFAULT_DEF (vuse)) code to be done solely when
vn_reference_lookup_2 is called directly from walk_non_aliased_vuses and not
when
called from vn_reference_lookup_3?

[Bug ipa/111444] [14 Regression] Wrong code at -O2/3/s on x86_64-gnu since r14-3226-gd073e2d75d9

[jakub at gcc dot gnu.org]

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
This vn_reference_lookup_2 call from vn_reference_lookup_3 has been added for
PR23094 in r8-4877-gd7a160a45ea7ed09.

[Bug ipa/111444] [14 Regression] Wrong code at -O2/3/s on x86_64-gnu since r14-3226-gd073e2d75d9

[jakub at gcc dot gnu.org]

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
But then vnresult->result of &a (what is correctly believed to be in *i at the
start of the function) is different from rhs of &e and so it doesn't return
res;
So, I'm afraid I don't know what's going on.

[Bug ipa/111444] [14 Regression] Wrong code at -O2/3/s on x86_64-gnu since r14-3226-gd073e2d75d9

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
I will have a look then.

[Bug ipa/111444] [14 Regression] Wrong code at -O2/3/s on x86_64-gnu since r14-3226-gd073e2d75d9

[rguenth at gcc dot gnu.org]

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

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
OK, so the issue is that we're recording the IPA result with the wrong VUSE
since we're calling vn_reference_lookup_2 with !data->last_vuse_ptr but
data->finish (vr->set, vr->base_set, v) inserts a hashtable entry with
data->last_vuse.  Note it's somewhat unexpected that vn_reference_lookup_2
performs hashtable insertion which is what causes the issue.  It's also
not as easy as using the updated vuse since if we're coming from translation
through a memcpy that would be wrong.  In fact we probably want to avoid
doing any insertion if theres sth fishy going on (!data->last_vuse_ptr).

The best fix would likely be to pre-insert all the IPA-CP known constants
instead of trying to discover them "late".

I'm testing the easy fix for now.

[Bug ipa/111444] [14 Regression] Wrong code at -O2/3/s on x86_64-gnu since r14-3226-gd073e2d75d9

[rguenth at gcc dot gnu.org]

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

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #8)
> The best fix would likely be to pre-insert all the IPA-CP known constants
> instead of trying to discover them "late".
> 
> I'm testing the easy fix for now.

Hmm.  gcc.dg/ipa/pr92497-1.c FAILs because of that.  We get

__attribute__((noinline))
int bar.constprop (struct a a)
{
  intD.6 a$aD.2808;
  intD.6 D.2807;
  struct a aD.2806;
  intD.6 _4;

  <bb 2> [local count: 1073741824]:
  # .MEM_5 = VDEF <.MEM_2(D)>
  aD.2806 = aD.2800;
  # VUSE <.MEM_5>
  a$a_3 = aD.2806.aD.2769;

here and thus translate through the aggregate copy - the result should then
be put on aD.2806 but of course only with .MEM_5.

Maybe we can and should always use the default def here but I'm slightly
uneasy with the ref adjustment, esp. since we're going to record
for the saved operands (if those exist - the path where it goes wrong
isn't translated).

[Bug ipa/111444] [14 Regression] Wrong code at -O2/3/s on x86_64-gnu since r14-3226-gd073e2d75d9

[rguenth at gcc dot gnu.org]

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

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
Hmm, I have another fix.

[Bug ipa/111444] [14 Regression] Wrong code at -O2/3/s on x86_64-gnu since r14-3226-gd073e2d75d9

[cvs-commit at gcc dot gnu.org]

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

--- Comment #11 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

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

commit r14-8655-gcfb3f666562fb4ab896a05c234a697afb63627a4
Author: Richard Biener <rguenther@suse.de>
Date:   Wed Jan 31 10:42:48 2024 +0100

    tree-optimization/111444 - avoid insertions when skipping defs

    The following avoids inserting expressions for IPA CP discovered
    equivalences into the VN hashtables when we are optimistically
    skipping may-defs in the attempt to prove it's redundant.

            PR tree-optimization/111444
            * tree-ssa-sccvn.cc (vn_reference_lookup_3): Do not use
            vn_reference_lookup_2 when optimistically skipping may-defs.

            * gcc.dg/torture/pr111444.c: New testcase.

[Bug ipa/111444] [14 Regression] Wrong code at -O2/3/s on x86_64-gnu since r14-3226-gd073e2d75d9

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #12 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.