Workaround ICE in gimple_static_chain_flags
Commit Message
Hi,
this patch workarounds ICE in gimple_static_chain_flags. I added a
sanity check that the nested function is never considered interposable
because such situation makes no sense: nested functions have no static
API and can not be safely merged across translation units.
It turns out however that this triggers for Ada and also for Fortran if
LTO partitioning separates nested function from its origin. The secon
is bug in binds_to_current_def_p which I was fixing some time ago but it
seems that the patch got lost :(
So I will dig it out and fix the situation property however to unbreak
periodic testers I am silencing the ICE for now (at expense of missed
optimization)
Honza
gcc/ChangeLog:
2021-11-04 Jan Hubicka <hubicka@ucw.cz>
PR ipa/103058
* gimple.c (gimple_call_static_chain_flags): Handle case when
nested function does not bind locally.
Comments
On Thu, Nov 04, 2021 at 05:13:41PM +0100, Jan Hubicka via Gcc-patches wrote:
> this patch workarounds ICE in gimple_static_chain_flags. I added a
> sanity check that the nested function is never considered interposable
> because such situation makes no sense: nested functions have no static
> API and can not be safely merged across translation units.
> It turns out however that this triggers for Ada and also for Fortran if
> LTO partitioning separates nested function from its origin. The secon
> is bug in binds_to_current_def_p which I was fixing some time ago but it
> seems that the patch got lost :(
Wouldn't the right fix be to ensure during partitioning that nested function
always goes into the same partition as its containing function?
Jakub
> On Thu, Nov 04, 2021 at 05:13:41PM +0100, Jan Hubicka via Gcc-patches wrote:
> > this patch workarounds ICE in gimple_static_chain_flags. I added a
> > sanity check that the nested function is never considered interposable
> > because such situation makes no sense: nested functions have no static
> > API and can not be safely merged across translation units.
> > It turns out however that this triggers for Ada and also for Fortran if
> > LTO partitioning separates nested function from its origin. The secon
> > is bug in binds_to_current_def_p which I was fixing some time ago but it
> > seems that the patch got lost :(
>
> Wouldn't the right fix be to ensure during partitioning that nested function
> always goes into the same partition as its containing function?
We are losing optimization because of binds_to_current_def_p not seing
through partitions in other cases too, so it would only help the nested
functoins and not other cases.
For example we may determine callee to not read gloal memory but we
apply this logic only if we know that callee will not be replaced by
semantically equivalent one that does (i.e. because it is not optimized
and contains dead global pointer dereference that may ICE).
Also in general we do not want to impose aritificial constrains to
partitioner.
I had patch that was explicitly storing binds_to_current_def flag to
cgraph nodes that should make this problem go away, but I need to look
it up (it is years old and I guess i forgot to commit it back then)
Honza
>
> Jakub
>
> On Thu, Nov 04, 2021 at 05:13:41PM +0100, Jan Hubicka via Gcc-patches wrote:
> > this patch workarounds ICE in gimple_static_chain_flags. I added a
> > sanity check that the nested function is never considered interposable
> > because such situation makes no sense: nested functions have no static
> > API and can not be safely merged across translation units.
> > It turns out however that this triggers for Ada and also for Fortran if
> > LTO partitioning separates nested function from its origin. The secon
> > is bug in binds_to_current_def_p which I was fixing some time ago but it
> > seems that the patch got lost :(
>
> Wouldn't the right fix be to ensure during partitioning that nested function
> always goes into the same partition as its containing function?
I did some more poking about this and I am not able to reproduce any
problems due to LTO partitioning: at the moment we bring symbol local we
set its resolution info which is later used by binds_to_current_def_p
and it seems to do the right thing (so I suppose I commited the patch
while ago after all).
However there are ices at compile time that are due to frontned
producing non-static nested functions that seems wrong to me...
Honza
>
> Jakub
>
@@ -1666,7 +1666,18 @@ gimple_call_static_chain_flags (const gcall *stmt)
int modref_flags = summary->static_chain_flags;
/* We have possibly optimized out load. Be conservative here. */
- gcc_checking_assert (node->binds_to_current_def_p ());
+ if (!node->binds_to_current_def_p ())
+ {
+ if ((modref_flags & EAF_UNUSED) && !(flags & EAF_UNUSED))
+ {
+ modref_flags &= ~EAF_UNUSED;
+ modref_flags |= EAF_NOESCAPE;
+ }
+ if ((modref_flags & EAF_NOREAD) && !(flags & EAF_NOREAD))
+ modref_flags &= ~EAF_NOREAD;
+ if ((modref_flags & EAF_DIRECT) && !(flags & EAF_DIRECT))
+ modref_flags &= ~EAF_DIRECT;
+ }
if (dbg_cnt (ipa_mod_ref_pta))
flags |= modref_flags;
}