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

--- Comment #11 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to rguent...@suse.de from comment #10)
> On Tue, 10 Dec 2024, pinskia at gcc dot gnu.org wrote:
> 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117965
> > 
> > --- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
> > Created attachment 59825 [details]
> >   --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=59825&action=edit
> > Fix
> > 
> > Note this includes debugging still and no testcases. And has only done some
> > quick testing on the testcases.
> 
> Another alternative is to not fail when !phivn_valid_p but to insert
> a dereference on the edge (IIRC you had a patch adjusting phiprop to
> consider not just ADDR_EXPRs).  I'd still require at least one
> ADDR_EXPR (or chained value) for the transform to be profitable
> (or at most one inserted deref).
> 
> Note the simplest fix, not using bb_has_eh_pred or so is to simply
> allow phivn[SSA_NAME_VERSION (name)].vuse == gimple_vuse of the
> original deref.  We find that only after checking all PHI args.
> We could possibly delay this VUSE match fallback to that phase.
> So maybe it's not the "simplest" fix.
> 
> bb_has_eh_pred requires the VUSE use to be in the EH receiver directly,
> not allowing for that receiver to flow to the VUSE by other means.
> 
> I also think that
> 
>   FOR_EACH_IMM_USE_STMT (use_stmt, ui2, vuse)
>     {
>       /* If BB does not dominate a VDEF, the value is invalid.  */
>       if ((gimple_vdef (use_stmt) != NULL_TREE 
>            || gimple_code (use_stmt) == GIMPLE_PHI)
>           && !dominated_by_p (CDI_DOMINATORS, gimple_bb (use_stmt), bb))
>         {   
>           ok = false;
>           break;
> 
> is the wrong way around - once we find a VUSE stmt that dominates BB
> the value is OK, not the other way around (find one that doesn't and
> it's invalid).  A PHI doesn't work there though (it works only at
> the relevant PHI arg defs source block, but there dominance can't
> work).

Hmm, no - I think I got this wrong.  That of course only works if the
VUSE is the same as the later VUSE (which we didn't determine yet).

But I'm back to possibly collecting all derefs beforehand and checking
against their VUSEs?  I'm not sure ignoring EH is OK since in principle
the try/catch could continue towards the later load.  That is,

  # PHI <...>
   .. = *first_load;

  non-call EH
    /  \
        EH VDEF
    \  /
      |
  # PHI <...>
  .. = *second_load;

so the EH VDEF is still between the loads, possibly invaldiating them.

We might want to simply use walk_aliased_vdefs here (but that wants to
start at the 2nd load VUSE as well), terminating at the def of the first
load VUSE.

Reply via email to