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. 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
