On Thu, 10 Aug 2023, Jeff Law wrote: > > > On 8/10/23 09:05, Richard Biener wrote: > > > > > >> Am 10.08.2023 um 17:01 schrieb Jeff Law via Gcc-patches > >> <gcc-patches@gcc.gnu.org>: > >> > >> > >> > >>> On 8/10/23 06:41, Richard Biener via Gcc-patches wrote: > >>> The following adjusts the heuristic when we perform PHI insertion > >>> during GIMPLE PRE from requiring at least one edge that is supposed > >>> to be optimized for speed to also doing insertion when the expression > >>> is available on all edges (but possibly with different value) and > >>> we'd at most have one copy from a constant. The first ensures > >>> we optimize two computations on all paths to one plus a possible > >>> copy due to the PHI, the second makes sure we do not need to insert > >>> many possibly large copies from constants, disregarding the > >>> cummulative size cost of the register copies when they are not > >>> coalesced. > >>> The case in the testcase is > >>> <bb 5> > >>> _14 = h; > >>> if (_14 == 0B) > >>> goto <bb 7>; > >>> else > >>> goto <bb 6>; > >>> <bb 6> > >>> h = 0B; > >>> <bb 7> > >>> h.6_12 = h; > >>> and we want to optimize that to > >>> <bb 7> > >>> # h.6_12 = PHI <_14(5), 0B(6)> > >>> If we want to consider the cost of the register copies I think the > >>> only simplistic enough way would be to restrict the special-case to > >>> two incoming edges - we'd assume one register copy is coalesced > >>> leaving one copy from a register or from a constant. > >>> As with every optimization the downstream effects are probably > >>> bigger than what we can locally estimate. > >>> Bootstrapped and tested on x86_64-unknown-linux-gnu. > >>> Any comments? > >>> Thanks, > >>> Richard. > >>> PR tree-optimization/110963 > >>> * tree-ssa-pre.cc (do_pre_regular_insertion): Also insert > >>> a PHI node when the expression is available on all edges > >>> and we insert at most one copy from a constant. > >>> * gcc.dg/tree-ssa/ssa-pre-34.c: New testcase. > >> The other thing in this space is to extend it to the case where multiple > >> phi args have the same constant. My recollection is we had some bits in > >> the out-of-ssa code to factor those into a single path -- if that still > >> works in the more modern expansion approach, the it'd likely be a win to > >> support as well. > > > > Yes, though it comes at the cost of another branch, no? The other thing to > > consider is that undoing the transform is much more difficult for constants, > > we usually have no idea what expression to re-materialize for it. > For this specific test, I don't think so. But in general I'm less sure. > > Yea, undoing is much more difficult for constants.
Given there are no further comments I have pushed the original patch, we'll see if there's any fallout or obvious opportunities left that we might want to enhance the heuristic for. Richard.
