[Bug ipa/97292] New: [11 Regression] dealII from SPECCPU 2016 no longer terminates after g:c34db4b6f8a5d80367c709309f9b00cb32630054

[Bug ipa/97292] New: [11 Regression] dealII from SPECCPU 2016 no longer terminates after g:c34db4b6f8a5d80367c709309f9b00cb32630054

[tnfchris at gcc dot gnu.org]

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

            Bug ID: 97292
           Summary: [11 Regression] dealII from SPECCPU 2016 no longer
                    terminates after
                    g:c34db4b6f8a5d80367c709309f9b00cb32630054
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: ipa
          Assignee: unassigned at gcc dot gnu.org
          Reporter: tnfchris at gcc dot gnu.org
                CC: hubicka at gcc dot gnu.org, marxin at gcc dot gnu.org
  Target Milestone: ---
            Target: aarch64-none-linux-gnu

With just -Ofast the benchmark doesn't seem to ever terminate until it is
eventually killed.

With -Ofast -flto it seems to give a runtime error.

Tweaking various params gets you one of the above behavior.

Bisected to g:c34db4b6f8a5d80367c709309f9b00cb32630054

[Bug ipa/97292] [11 Regression] dealII from SPECCPU 2016 no longer terminates after g:c34db4b6f8a5d80367c709309f9b00cb32630054

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
           Priority|P3                          |P1
           Assignee|unassigned at gcc dot gnu.org      |hubicka at gcc dot gnu.org
             Blocks|                            |26163
      Known to fail|                            |11.0
               Host|                            |x86_64-linux-gnu
   Last reconfirmed|                            |2020-10-06
   Target Milestone|---                         |11.0
      Known to work|                            |10.2.0
             Status|UNCONFIRMED                 |ASSIGNED

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
Confirmed also on x86_64-linux-gnu.


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=26163
[Bug 26163] [meta-bug] missed optimization in SPEC (2k17, 2k and 2k6 and 95)

[Bug ipa/97292] [11 Regression] dealII from SPECCPU 2016 no longer terminates after g:c34db4b6f8a5d80367c709309f9b00cb32630054

[marxin at gcc dot gnu.org]

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

--- Comment #2 from Martin Liška <marxin at gcc dot gnu.org> ---
Created attachment 49314
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49314&action=edit
Debug counter patch

First bad debug counter value is -fdbg-cnt=ipa_mod_ref:2793.
Difference is for: TriaObjectAccessor<3, 3>::operator++():

  LTO stores:
    Limits: 32 bases, 16 refs
      Base 0:struct TriaObjectAccessor (alias set 172)
        Ref 0:int (alias set 6)
          access: Parm 0
          access: Parm 0 param offset:0 offset:0 size:32 max_size:32

  LTO stores:
    Limits: 32 bases, 16 refs
      Base 0:struct TriaObjectAccessor (alias set 172)
        Ref 0:int (alias set 6)
          access: Parm 0 param offset:0 offset:32 size:32 max_size:32
          access: Parm 0 param offset:0 offset:0 size:32 max_size:32

template <int dim>
inline
void
TriaObjectAccessor<3,dim>::operator ++ ()
{
  ++this->present_index;
                                   // is index still in the range of
                                   // the vector?
  while (this->present_index
         >=
        
static_cast<int>(this->tria->levels[this->present_level]->hexes.hexes.size()))
    {
                                       // no -> go one level up
      ++this->present_level;
      this->present_index = 0;
                                       // highest level reached?
      if (this->present_level >= static_cast<int>(this->tria->levels.size()))
        {
                                           // return with past the end pointer
          this->present_level = this->present_index = -1;
          return;
        };
    };
}

hope it helps.

[Bug ipa/97292] [11 Regression] dealII from SPECCPU 2016 no longer terminates after g:c34db4b6f8a5d80367c709309f9b00cb32630054

[hubicka at ucw dot cz]

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

--- Comment #3 from Jan Hubicka <hubicka at ucw dot cz> ---
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97292
> 
> --- Comment #2 from Martin Liška <marxin at gcc dot gnu.org> ---
> Created attachment 49314
>   --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49314&action=edit
> Debug counter patch
> 
> First bad debug counter value is -fdbg-cnt=ipa_mod_ref:2793.
> Difference is for: TriaObjectAccessor<3, 3>::operator++():
> 
>   LTO stores:
>     Limits: 32 bases, 16 refs
>       Base 0:struct TriaObjectAccessor (alias set 172)
>         Ref 0:int (alias set 6)
>           access: Parm 0
>           access: Parm 0 param offset:0 offset:0 size:32 max_size:32
> 
>   LTO stores:
>     Limits: 32 bases, 16 refs
>       Base 0:struct TriaObjectAccessor (alias set 172)
>         Ref 0:int (alias set 6)
>           access: Parm 0 param offset:0 offset:32 size:32 max_size:32
>           access: Parm 0 param offset:0 offset:0 size:32 max_size:32

Thanks a lot for trakcing it down.  At the first glance, the summary
looks correct.  I hope this is just another manifestation of the bug
about mem_ref offsets, but I will take a lok.

Pehraps it would be more useful to add debug counter to the place
disambiguation is done (where we dump ipa-modref: in tree-ssa-alias.
That way we would have both the summary and the access.

Honza
> 
> template <int dim>
> inline
> void
> TriaObjectAccessor<3,dim>::operator ++ ()
> {
>   ++this->present_index;
>                                    // is index still in the range of
>                                    // the vector?
>   while (this->present_index
>          >=
>         
> static_cast<int>(this->tria->levels[this->present_level]->hexes.hexes.size()))
>     {
>                                        // no -> go one level up
>       ++this->present_level;
>       this->present_index = 0;
>                                        // highest level reached?
>       if (this->present_level >= static_cast<int>(this->tria->levels.size()))
>         {
>                                            // return with past the end pointer
>           this->present_level = this->present_index = -1;
>           return;
>         };
>     };
> }
> 
> hope it helps.
> 
> -- 
> You are receiving this mail because:
> You are the assignee for the bug.
> You are on the CC list for the bug.

[Bug ipa/97292] [11 Regression] dealII from SPECCPU 2016 no longer terminates after g:c34db4b6f8a5d80367c709309f9b00cb32630054

[marxin at gcc dot gnu.org]

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

--- Comment #4 from Martin Liška <marxin at gcc dot gnu.org> ---
> Pehraps it would be more useful to add debug counter to the place
> disambiguation is done (where we dump ipa-modref: in tree-ssa-alias.
> That way we would have both the summary and the access.

Please finish my dbgcnt patch, I see multiple places in the file which were
added in context of IPA MOD REF.

[Bug ipa/97292] [11 Regression] dealII from SPECCPU 2016 no longer terminates after g:c34db4b6f8a5d80367c709309f9b00cb32630054

[rguenth at gcc dot gnu.org]

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

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
It also reproduces on Haswell x86_64 (czery).

Re: [Bug ipa/97292] [11 Regression] dealII from SPECCPU 2016 no longer terminates after g:c34db4b6f8a5d80367c709309f9b00cb32630054

[Jan Hubicka]

Hi,
the following patch should let us to pinpoint the wrong disambiguation.
With -fdump-tree-all-details we should also see the difference in dump
file.

Honza

diff --git a/gcc/dbgcnt.def b/gcc/dbgcnt.def
index cf8775b2b66..07946a85ecc 100644
--- a/gcc/dbgcnt.def
+++ b/gcc/dbgcnt.def
@@ -171,6 +171,7 @@ DEBUG_COUNTER (if_after_reload)
 DEBUG_COUNTER (if_conversion)
 DEBUG_COUNTER (if_conversion_tree)
 DEBUG_COUNTER (ipa_cp_bits)
+DEBUG_COUNTER (ipa_mod_ref)
 DEBUG_COUNTER (ipa_sra_params)
 DEBUG_COUNTER (ipa_sra_retvalues)
 DEBUG_COUNTER (ira_move)
diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
index 97dc4ac8814..ba208604c30 100644
--- a/gcc/tree-ssa-alias.c
+++ b/gcc/tree-ssa-alias.c
@@ -2470,6 +2470,9 @@ modref_may_conflict (const gimple *stmt,
   if (tt->every_base)
     return true;
 
+  if (!dbg_cnt (ipa_mod_ref))
+    return true;
+
   base_set = ao_ref_base_alias_set (ref);
 
   ref_set = ao_ref_alias_set (ref);

[Bug ipa/97292] [11 Regression] dealII from SPECCPU 2016 no longer terminates after g:c34db4b6f8a5d80367c709309f9b00cb32630054

[hubicka at ucw dot cz]

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

--- Comment #6 from Jan Hubicka <hubicka at ucw dot cz> ---
Hi,
the following patch should let us to pinpoint the wrong disambiguation.
With -fdump-tree-all-details we should also see the difference in dump
file.

Honza

diff --git a/gcc/dbgcnt.def b/gcc/dbgcnt.def
index cf8775b2b66..07946a85ecc 100644
--- a/gcc/dbgcnt.def
+++ b/gcc/dbgcnt.def
@@ -171,6 +171,7 @@ DEBUG_COUNTER (if_after_reload)
 DEBUG_COUNTER (if_conversion)
 DEBUG_COUNTER (if_conversion_tree)
 DEBUG_COUNTER (ipa_cp_bits)
+DEBUG_COUNTER (ipa_mod_ref)
 DEBUG_COUNTER (ipa_sra_params)
 DEBUG_COUNTER (ipa_sra_retvalues)
 DEBUG_COUNTER (ira_move)
diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
index 97dc4ac8814..ba208604c30 100644
--- a/gcc/tree-ssa-alias.c
+++ b/gcc/tree-ssa-alias.c
@@ -2470,6 +2470,9 @@ modref_may_conflict (const gimple *stmt,
   if (tt->every_base)
     return true;

+  if (!dbg_cnt (ipa_mod_ref))
+    return true;
+
   base_set = ao_ref_base_alias_set (ref);

   ref_set = ao_ref_alias_set (ref);

[Bug ipa/97292] [11 Regression] dealII from SPECCPU 2016 no longer terminates after g:c34db4b6f8a5d80367c709309f9b00cb32630054

[marxin at gcc dot gnu.org]

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

--- Comment #7 from Martin Liška <marxin at gcc dot gnu.org> ---
Ok, I've just installed the patch with the debug counter.

I can reproduce it with first bad value with --param=lto-partitions=1:
-fdbg-cnt=ipa_mod_ref:16848

and the difference comes from dealII.ltrans0.ltrans.137t.pre:

distribute_dofs (struct DoFHandler * const this, const struct FiniteElement &
ff, const unsigned int offset):

GOOD:
  <bb 164> [local count: 410235502]:
  # __n_981 = PHI <__n_979(163), __n_978(162)>
  # __tmp$_M_p_984 = PHI <_980(163), _977(162)>
  _982 = (unsigned int) __n_981;
  _983 = 1 << _982;
  _985 = *__tmp$_M_p_984;
  _986 = _983 | _985;
  *__tmp$_M_p_984 = _986;
  operator++ (&cell);
  _25 = BIT_FIELD_REF <MEM[(const struct TriaAccessor *)&cell + 8B], 64, 0>;
  if (_25 != -1)
    goto <bb 165>; [89.00%]
  else
    goto <bb 166>; [11.00%]

  <bb 165> [local count: 365109597]:
  pretmp_2354 = MEM[(int *)&cell + 12B];
  goto <bb 132>; [100.00%]

  <bb 166> [local count: 50703262]:
  # next_free_dof_577 = PHI <offset_13(D)(128), next_free_dof_822(164)>
  this_6(D)->used_dofs = next_free_dof_577;
  MEM[(struct _Bvector_impl_data *)&tmp] ={v} {CLOBBER};
  MEM[(struct __as_base  &)&tmp] ={v} {CLOBBER};

BAD:
  <bb 164> [local count: 410235502]:
  # __n_981 = PHI <__n_979(163), __n_978(162)>
  # __tmp$_M_p_984 = PHI <_980(163), _977(162)>
  _982 = (unsigned int) __n_981;
  _983 = 1 << _982;
  _985 = *__tmp$_M_p_984;
  _986 = _983 | _985;
  *__tmp$_M_p_984 = _986;
  operator++ (&cell);
  _25 = BIT_FIELD_REF <MEM[(const struct TriaAccessor *)&cell + 8B], 64, 0>;
  if (_25 != -1)
    goto <bb 132>; [89.00%]
  else
    goto <bb 165>; [11.00%]

  <bb 165> [local count: 50703262]:
  # next_free_dof_577 = PHI <offset_13(D)(128), next_free_dof_822(164)>
  this_6(D)->used_dofs = next_free_dof_577;
  MEM[(struct _Bvector_impl_data *)&tmp] ={v} {CLOBBER};
  MEM[(struct __as_base  &)&tmp] ={v} {CLOBBER};

[Bug ipa/97292] [11 Regression] dealII from SPECCPU 2016 no longer terminates after g:c34db4b6f8a5d80367c709309f9b00cb32630054

[marxin at gcc dot gnu.org]

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

--- Comment #8 from Martin Liška <marxin at gcc dot gnu.org> ---
Created attachment 49329
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49329&action=edit
Good PRE dump file

[Bug ipa/97292] [11 Regression] dealII from SPECCPU 2016 no longer terminates after g:c34db4b6f8a5d80367c709309f9b00cb32630054

[marxin at gcc dot gnu.org]

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

--- Comment #9 from Martin Liška <marxin at gcc dot gnu.org> ---
Created attachment 49330
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49330&action=edit
Bad PRE dump file

[Bug ipa/97292] [11 Regression] dealII from SPECCPU 2016 no longer terminates after g:c34db4b6f8a5d80367c709309f9b00cb32630054

[marxin at gcc dot gnu.org]

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

--- Comment #10 from Martin Liška <marxin at gcc dot gnu.org> ---
Good PRE dump file with details:
https://drive.google.com/file/d/114AAhbNKeiJ7w3FYOcumdpnnMCNvdjiy/view?usp=sharing

Bad PRE dump file with details:
https://drive.google.com/file/d/14_dV-FYSe1j0Sx-zIc8d_6ypuFcmPnZl/view?usp=sharing

[Bug ipa/97292] [11 Regression] dealII from SPECCPU 2006 no longer terminates after g:c34db4b6f8a5d80367c709309f9b00cb32630054

[marxin at gcc dot gnu.org]

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

--- Comment #11 from Martin Liška <marxin at gcc dot gnu.org> ---
(In reply to Tamar Christina from comment #0)
> With just -Ofast the benchmark doesn't seem to ever terminate until it is
> eventually killed.
> 

Can't reproduce the stuck without -flto.

[Bug ipa/97292] [11 Regression] dealII from SPECCPU 2006 no longer terminates after g:c34db4b6f8a5d80367c709309f9b00cb32630054

[tnfchris at gcc dot gnu.org]

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

--- Comment #12 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
(In reply to Martin Liška from comment #11)
> (In reply to Tamar Christina from comment #0)
> > With just -Ofast the benchmark doesn't seem to ever terminate until it is
> > eventually killed.
> > 
> 
> Can't reproduce the stuck without -flto.

You're right, had the timeout set too low in the CI. it does work without lto.
Sorry about that.

[Bug ipa/97292] [11 Regression] dealII from SPECCPU 2006 no longer terminates after g:c34db4b6f8a5d80367c709309f9b00cb32630054

[hubicka at gcc dot gnu.org]

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

--- Comment #13 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
OK, I can reproduce it even though the propagation happens in different
function in dom rather than pre.
The callee is again the operator++ so I start to suspect aliasing violation
there.

I get:
ipa-modref: in build_patches.constprop/375859, call to operator++/33005 does
not clobber MEM <struct DoFHandler * const> [(struct DoFCellAccessor
*)&active_cell] 6->6

parameter of call is &active_cell so we disambiguate param0 with 

while the summary for ipa-modref

  stores:                                                                       
    Limits: 32 bases, 16 refs                                                   
      Base 0: alias set 11                                                      
        Ref 0: alias set 13                                                     
          access: Parm 0 param offset:64 offset:32 size:32 max_size:32          
          access: Parm 0 param offset:64 offset:0 size:32 max_size:32           

So this really seems that the alias set 6 is not conflicting with  alias sets
11 or 13.

Honza

[Bug ipa/97292] [11 Regression] dealII from SPECCPU 2006 no longer terminates after g:c34db4b6f8a5d80367c709309f9b00cb32630054

[hubicka at gcc dot gnu.org]

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

--- Comment #14 from Jan Hubicka <hubicka at gcc dot gnu.org> ---

So this really seems that the alias set 6 is not conflicting with  alias sets
11 or 13. active_cell is
struct active_cell_iterator active_cell;
and the code around seems SRA injected
  MEM[(struct TriaRawIterator *)&active_cell] ={v} {CLOBBER};                   
  MEM <struct DoFHandler *> [(struct DoFCellAccessor *)&active_cell] =
cell$dof_handler_253;
  MEM <int> [(struct DoFCellAccessor *)&active_cell + 8B] =
cell$8$present_level_242;
  MEM <int> [(struct DoFCellAccessor *)&active_cell + 12B] =
cell$8$present_index_245;
  MEM <const struct Triangulation *> [(struct DoFCellAccessor *)&active_cell +
16B] = cell$8$tria_244;
  operator++ (&active_cell);                                                    
  MEM[(struct TriaRawIterator *)&D.17355].accessor = MEM[(const struct
TriaRawIterator &)&active_cell].accessor;

and we CSE is acrss the call

Honza

[Bug ipa/97292] [11 Regression] dealII from SPECCPU 2006 no longer terminates after g:c34db4b6f8a5d80367c709309f9b00cb32630054

[hubicka at gcc dot gnu.org]

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

--- Comment #15 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
So it seems that problem is that store in operator++ is TriaObjectAcessor while
read is DoFCellAccessor. I am however not sure where the type puning happens :(

[Bug ipa/97292] [11 Regression] dealII from SPECCPU 2006 no longer terminates after g:c34db4b6f8a5d80367c709309f9b00cb32630054

[marxin at gcc dot gnu.org]

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

--- Comment #16 from Martin Liška <marxin at gcc dot gnu.org> ---
I was able to find a minimal debug counter for IPA modref that triggers that:
-fdbg-cnt=ipa_mod_ref:10902-10904

[Bug ipa/97292] [11 Regression] dealII from SPECCPU 2006 no longer terminates after g:c34db4b6f8a5d80367c709309f9b00cb32630054

[hubicka at gcc dot gnu.org]

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

--- Comment #17 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
And is it again the operator++ triggering the problem?
It looks like aliasing bug to me, but in a template hell and
-Wstrict-aliasing=3 is happy.

The reason why parameter tracking is necessary seems to be the fact that we
figure out some accesses from inner calls are local to the ++ operator and do
not consider them in a summary (that is correct)

[Bug ipa/97292] [11 Regression] dealII from SPECCPU 2006 no longer terminates after g:c34db4b6f8a5d80367c709309f9b00cb32630054

[marxin at gcc dot gnu.org]

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

--- Comment #18 from Martin Liška <marxin at gcc dot gnu.org> ---
(In reply to Jan Hubicka from comment #17)
> And is it again the operator++ triggering the problem?
> It looks like aliasing bug to me, but in a template hell and
> -Wstrict-aliasing=3 is happy.

It's another place where a *(&cell + 12) is moved by PRE:

   <bb 167> [local count: 365109597]:
   pretmp_2371 = MEM[(const struct Triangulation * *)&cell + 16B];
   pretmp_2373 = MEM[(int *)&cell + 8B];
-  pretmp_2375 = MEM[(int *)&cell + 12B];
   goto <bb 131>; [100.00%]


> 
> The reason why parameter tracking is necessary seems to be the fact that we
> figure out some accesses from inner calls are local to the ++ operator and
> do not consider them in a summary (that is correct)

[Bug ipa/97292] [11 Regression] dealII from SPECCPU 2006 no longer terminates after g:c34db4b6f8a5d80367c709309f9b00cb32630054

[cvs-commit at gcc dot gnu.org]

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

--- Comment #19 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Martin Liska <marxin@gcc.gnu.org>:

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

commit r11-3748-gffe8baa996486fa0313aa804a064a58b0b161f07
Author: Jan Hubicka <jh@suse.cz>
Date:   Fri Oct 9 11:29:58 2020 +0200

    IPA modref: fix miscompilation in clone when IPA modref is used

    gcc/ChangeLog:

            PR ipa/97292
            PR ipa/97335
            * ipa-modref-tree.h (copy_from): Drop summary in a
            clone.

[Bug ipa/97292] [11 Regression] dealII from SPECCPU 2006 no longer terminates after g:c34db4b6f8a5d80367c709309f9b00cb32630054

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

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

--- Comment #20 from Martin Liška <marxin at gcc dot gnu.org> ---
Fixed now.