On Fri, Jan 24, 2025 at 5:43 AM Alexandre Oliva <ol...@adacore.com> wrote: > > On Jan 23, 2025, Richard Biener <richard.guent...@gmail.com> wrote: > > > That said, it'd be a lot clearer if this simply read > > > || !access_in_bounds_of_type_p (TREE_TYPE (inner), bs, bp) > > > without all the other weird conditions. > > ACK, will do. > > >> + /* Check that the loads that we're trying to combine have the same vuse > >> and > >> + same trapping status. */ > >> if ((ll_load && rl_load) > >> - ? gimple_vuse (ll_load) != gimple_vuse (rl_load) > >> + ? (gimple_vuse (ll_load) != gimple_vuse (rl_load) > >> + || (tree_could_trap_p (gimple_assign_rhs1 (ll_load)) > >> + != tree_could_trap_p (gimple_assign_rhs1 (rl_load)))) > > > So that's to make the above added assert happy, right? > > No, this is to avoid introducing similar errors to the one we've just fixed. > > Consider this scenario: we're considering combining two separate loads > into one. They both load from the same word, but one can trap and the > other can't (say, because it's marked as nontrapping because it's > dominated by the trapping load from the same word). > > Without the condition above, there's a risk that we'll insert the merged > load next to the nontrapping load. But then, when both original loads > get deleted as dead, only a nontrapping load will remain, but the > dominating load that made it nontrapping won't be there any more, so the > nontrapping load could be moved/hoisted.
Hmm. I think when an original ref could trap that should be the insertion point (or the original ref should post-dominate the insertion point). It's fine when an originally may-trap reference becomes not trapping (and it really cannot trap). Introducing new (may-)traps is never OK. So, can we split the patch to the case with the testcase at hand and consider the assert and the extra tree_could_trap_p checks separately? Thanks, Richard. > But yeah, now that you mentioned, it's also needed to satisfy the > assert, because we could pick the wrong load to replace, and then we'd > get a mismatch. > > -- > Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ > Free Software Activist GNU Toolchain Engineer > More tolerance and less prejudice are key for inclusion and diversity > Excluding neuro-others for not behaving ""normal"" is *not* inclusive
