On Sat, Oct 19, 2024 at 5:08 PM Jeff Law <jeffreya...@gmail.com> wrote: > > > > On 10/18/24 7:41 PM, Andrew Pinski wrote: > > Sometimes factor_out_conditional_operation can factor out > > an operation that causes a phi node to become the same element. > > Other times, we want to factor out a binary operator because > > it can improve code generation, an example is PR 110015 (openjpeg). > > > > Note this includes a heuristic to decide if factoring out the operation > > is profitable or not. It can be expanded to include a better live range > > extend detector. Right now it has a simple one where if it is live on a > > dominating path, it is considered a live or if there are a small # of > > assign statements (defaults to 5), then it does not extend the live range > > too much. > > > > Bootstrapped and tested on x86_64-linux-gnu. > > > > PR tree-optimization/112418 > > > > gcc/ChangeLog: > > > > * tree-ssa-phiopt.cc (is_factor_profitable): New function. > > (factor_out_conditional_operation): Add merge argument. Remove > > arg0/arg1 arguments. Return bool instead of the new phi. > > Early return for virtual ops. Call is_factor_profitable to > > check if the factoring would be profitable. > > (pass_phiopt::execute): Call factor_out_conditional_operation > > on all phis instead of just singleton phi. > > * doc/invoke.texi (--param phiopt-factor-max-stmts-live=): Document. > > * params.opt (--param=phiopt-factor-max-stmts-live=): New opt. > > > > gcc/testsuite/ChangeLog: > > > > * gcc.dg/tree-ssa/factor_op_phi-1.c: New test. > > * gcc.dg/tree-ssa/factor_op_phi-2.c: New test. > > * gcc.dg/tree-ssa/factor_op_phi-3.c: New test. > > * gcc.dg/tree-ssa/factor_op_phi-4.c: New test. > Dregding up all kinds of old memories. > > Not terribly happy with the new param, but a magic constant would > probably be even worse.. So I won't object to that.
A heuristic based on GIMPLE stmt placement is quite bad (given TER). The constant is OK as a compile-time limiter though. The heuristic to not move across a call looks good - we might want to see whether we can expose a predicate that identifies "TER barriers"? I'll also note sinking has similar profitability issues. Moving a stmt downwards incurs possible extension of the lifetime of its SSA uses and it always shrinks the lifetime of its SSA defs. For the purpose of GIMPLE the granularity at which we'd want to compute live-in/out is those "TER sub-basic blocks" (and when TER is gone sub-basic blocks formed by spill points like calls). It looks like the PHI-OPT heuristic only looks at either the defs or the uses? Richard. > OK for the trunk. I wouldn't be terribly surprised if we see light > fallout on some of the crosses, but I don't think that should inherently > block the patch, we'll just fix them up. > > Jeff >
