Message ID | 20211104161341.GA83757@kam.mff.cuni.cz |
---|---|
State | New |
Headers | show |
Series | Workaround ICE in gimple_static_chain_flags | expand |
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 >
diff --git a/gcc/gimple.c b/gcc/gimple.c index 76768c19c8e..7a578f5113e 100644 --- a/gcc/gimple.c +++ b/gcc/gimple.c @@ -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; }