On Tue, Feb 8, 2022 at 9:58 PM Andrew MacLeod <amacl...@redhat.com> wrote: > > On 2/8/22 03:32, Richard Biener wrote: > > On Tue, Feb 8, 2022 at 2:33 AM Andrew MacLeod via Gcc-patches > > <gcc-patches@gcc.gnu.org> wrote: > >> On 2/7/22 09:29, Andrew MacLeod wrote: > >>> I have a proposal for PR 104288. > >>> > >>> ALL patches (in sequence) bootstrap on their own and each cause no > >>> regressions. > >> I've been continuing to work with this towards the GCC13 solution for > >> statement side effects. And There is another possibility we could > >> pursue which is less intrusive > >> > >> I adapted the portions of patch 2/4 which process nonnull, but changes > >> it from being in the nonnull class to being in the cache. > >> > >> THe main difference is it continues to use a single bit, just changing > >> all uses of it to *always* mean its non-null on exit to the block. > >> > >> Range-on-entry is changed to only check dominator blocks, and > >> range_of_expr doesn't check the bit at all.. in both ranger and the cache. > >> > >> When we are doing the block walk and process side effects, the on-entry > >> *cache* range for the block is adjusted to be non-null instead. The > >> statements which precede this stmt have already been processed, and all > >> remaining statements in this block will now pick up this new non-value > >> from the on-entry cache. This should be perfectly safe. > >> > >> The final tweak is when range_of_expr is looking the def block, it > >> normally does not have an on entry cache value. (the def is in the > >> block, so there is no entry cache value). It now checks to see if we > >> have set one, which can only happen when we have been doing the > >> statement walk and set the on-entry cache with a non-null value. This > >> allows us to pick up the non-null range in the def block too... (once we > >> have passed a statement which set nonnull of course). > >> > >> THis patch provides much less change to trunk, and is probably a better > >> approach as its closer to what is going to happen in GCC13. > >> > >> Im still working with it, so the patch isnt fully refined yet... but it > >> bootstraps and passes all test with no regressions. And passes the new > >> tests too. I figured I would mention this before you look to hard at > >> the other patchset. the GCC11 patch doesn't change. > >> > >> Let me know if you prefer this.... I think I do :-) less churn, and > >> closer to end state. > > Yes, I very much prefer this - some comments to the other patches > > still apply to this one. Like using get_nonnull_args and probably > > adding a bail-out to computing ranges from stmts that can throw > > internally or have abnormal control flow (to not get into range-on-exit > > vs. normal vs. exceptional or abnormal edges). > > > > Richard. > > with some minor performance tweaks, such as moving adjust_range() to the > header so it can be inlined . > > range-on-edge now applies the non-null from the src block if > appropriate, not in range-on-exit. That should resolve the internal > throwing statements I think. and I have switched over to > get_nonnull_args(). > > Bootstraps on build-x86_64-pc-linux-gnu and passes all regressions. > > OK for trunk, or did I miss something?
OK. I do think there's some confusion about -fnon-call-exceptions. The comment + // Non-call exceptions mean we could throw in the middle of the + // block, so just punt on those for now. also applies to regular exceptions, non-call vs. call EH just adds the possibility of non-call stmts to throw. I do not think that value-range propagation needs to care about stmts that throw in the middle of a block (which automatically means no EH edges and thus !stmt_can_throw_internal). When there are EH edges then restrictions apply to both calls and non-calls that throw. So whatever made those cfun->can_throw_non_call_exceptions necessary should have shown a more general issue with EH. That's something to look at, but not in the scope of this fix. Richard. > Andrew > > PS. odd.. I haven't seen the git diff be wrong before, but it shows the > ranger_cache::range_on_edge changes as being in > gimple_cache::range_of_expr... They are most definitely are not.... > and it applies/de-applies fine.. so its just an oddity I guess. >
