On Tue, Sep 29, 2020 at 3:07 PM Alexandre Oliva <ol...@adacore.com> wrote: > > On Sep 29, 2020, Richard Biener <richard.guent...@gmail.com> wrote: > > > On Tue, Sep 29, 2020 at 9:23 AM Alexandre Oliva <ol...@adacore.com> wrote: > > >> On Sep 28, 2020, Richard Biener <richard.guent...@gmail.com> wrote: > > > ifcombine should stop using fold*, yeah > > Wow, that's quite a lot of work for no expected improvement in codegen. > I don't expect to be able to justify such an undertaking :-( > > > I also think it will not end up using the simplifications using loads. > > Yeah, ifcombine's bb_no_side_effects_p gives up on any gimple_vuse in > the inner block. that won't do when the whole point is to merge loads > from memory. > > That seems excessive. Since we rule out any memory-changing side > effects, I suppose we could get away with checking for volatile operands > there. Then, adding just a little SSA_DEF chasing, I believe I could > bring all of the fold_truth_andor_1 logic I've worked on into ifcombine > without much difficulty, and then we could do away with at least that > part of fold_truth_andor.
The current restrictions were for sure to make my life easier at start when implementing the pass ;) Note that you have to watch out for short-circuited stmts that may trap or invoke undefined behavior at runtime. > > Specifically your patch seems to introduce splitting of loads > > at alignment boundaries > > ... when there's another compare involving a load from either side of > the crossed alignment boundary. Even on arches that can do unaligned > loads, the result is no worse, and if there are multiple fields crossing > consecutive alignment boundaries, the codegen and performance difference > can be pretty significant. Ah, OK - I didn't look that closely. > >> I *think* ifcombine could even be extended so as to reuse the > >> separate-test logic I put in, by looking for non-immediate dominating > >> outer conditions for the inner condition. A further modified version of > >> fold_truth_andor_1 could then be used to combine the separate tests. > > > I think the structure of ifcombine doesn't exactly match what > > fold_truth_andor does > > How so? AFAICT ifcombine_ifandif deals exactly with the (gimplified > version of the) structure I described in the patch that started the > thread: > > (a.x1 EQNE b.x1) ANDOR (a.y1 EQNE b.y1) Indeed. > > -- > Alexandre Oliva, happy hacker > https://FSFLA.org/blogs/lxo/ > Free Software Activist > GNU Toolchain Engineer
