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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hubicka at gcc dot gnu.org,
                   |                            |rguenth at gcc dot gnu.org

--- Comment #16 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Martin Jambor from comment #14)
> Another option, which does not create an inter-pass dependency and
> does not clutter tree-inline any more, but which pessimizes IPA-SRA
> (put perhaps just alittle bit?), is making sure that the statements
> which might be left behind are harmless:
> 
> diff --git a/gcc/ipa-sra.c b/gcc/ipa-sra.c
> index 31de527d111..df54b98759c 100644
> --- a/gcc/ipa-sra.c
> +++ b/gcc/ipa-sra.c
> @@ -859,7 +859,8 @@ isra_track_scalar_value_uses (cgraph_node *node, tree
> name, int parm_num,
>             }
>           res += all_uses;
>         }
> -      else if ((is_gimple_assign (stmt) && !gimple_has_volatile_ops (stmt))
> +      else if ((is_gimple_assign (stmt) && !gimple_has_volatile_ops (stmt)
> +               && !gimple_could_trap_p (stmt))
>                || gimple_code (stmt) == GIMPLE_PHI)
>         {
>           tree lhs;
> 
> I'll see what a tree-inline.c solution would look like and then decide
> which of these I'll propose.

Any progress here?  I think using the above would be best.  Another
option would be, during inlining, to make sure that removed parameters
get initialized (that also silences the warning), just the constant to use
isn't 100% obvious.  I'd settle for a 1 which is safe for all cases I
can think of besides array indexing or pointer adjustment and for
signed operations where anything but 0 might cause some undefined overflow.
Note the overflow thing also means that the gimple_could_trap_p check
is not sufficient.

But in the end I always wonder whether those "premature" optimizations
during IPA analysis are worth it and why we not simply consider all uses
as "uses"?

I also think that the user should not be able to disable all of DCE
(or enable IPA SRA ontop of -O0).  But of course that only makes the
attack surface smaller - I can very well imagine "bad" circumstances
with us arriving at a similarly problematic IL before IPA SRA
analysis with plain -O2.

Oh, instead of initializing dropped parameters the inliner could also
make sure to map it to a NaT, dropping stmts that use that and turn
their defs into NaTs as well.  Of course then IPA SRAs "DCE" decision
becomes a correctness thing.

That said, a proper solution _will_ need to get rid of the dead stmts
at IPA transform time.

Not sure if we really need an "improper" solution right now (given
the testcase needs to disable DCE).  The situation here is similar
to that with inline predicates and __builtin_constant_p.

Reply via email to