[Bug ipa/65270] [5 regression] ICF needs to match TYPE attributes on memory accesses

["rguenth at gcc dot gnu.org" <gcc-bugzilla at gcc dot gnu dot org>]

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

--- Comment #11 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Jan Hubicka from comment #9)
> Lets reopen this.  I agree with other type flags - they seem safe except for
> TYPE_ATTRIBUTES of functions I will add compare of today.
> 
> > No.  But you can't compare restrict qualification by looking at a memory
> > reference pair from function A and function B.  You'd have to compare
> > all data dependences somehow.
> >
> > You could get some cases by comparing function parameter types specially
> > (but global vars also have an issue, as well as decl-by-reference
> > aggregate parameters).
> 
> Would it be enough to
>  - require match on arguments/return value types (that should handle
> function parameters as well as 
>  - require match on all global variable types

It would be a start, but no - consider early inlining and

static inline int foo (int * restrict p, int * restrict q) { ... }
static inline int foo2 (int *p, int *q) { same as foo }
int bar (int *p, int *q)
{
  return foo (p, q);
}
int bar2 (int *p, int *q)
{
  return foo2 (p, q);
}

where actual code generation difference would only show up in the main
optimization queue.

With the present implementation you'd have to compare MR_DEPENDENCE_CLIQUE
and MR_DEPENDENCE_BASE for MEM_REFs/TARGET_MEM_REFs.  Of course literally
comparing them isn't really correct (though if they are equal you are safe).

So - compare TYPE_MAIN_VARIANT of global var and function parameter types
and MR_DEPENDENCE_* on MEM_REFs should be enough to catch restrict
differences (fingers crossing ;)).

I'll try to cook up a patch.

> > It seems to me that ICF isn't quite ready for prime time.  Let's disable
> > it by default for now, ok?
> 
> I also read the discussion on IRC and discussed with Jeff.
> ICF has triggered interesting issues in alias/thunks areas. For a first time
> we output a lot of non-MI thunks on main targets and expand_thunk was
> getting things wrong in a nasty ways.  ICF also produce a lot of aliases and
> excercise symbol table code otherwise rarely used. Here I think it is
> valuable we fixed those bugs that were semi-latent for years.  I believe we
> are approaching steady state - at the moment we do not have any other ICF
> related PR open. Jeff suggested to wait 48 hours.
> 
> This PR is a result of my code audit I did over weekend. I fully leave the
> decision with you. I agree ICF is causing a lot of issues late in release
> and disabling it is a safe option. On the other hand I would be happy to
> work to get it fixed and working - it is quite useful optimization.

Sure - it just seems with every change you introduce bootstrap failures
on some targets which is quite bad.  Let's hope things stabilize and please
no re-writes anymore.