On Wed, Aug 20, 2025 at 2:16 PM Richard Sandiford
<richard.sandif...@arm.com> wrote:
>
> Richard Biener <richard.guent...@gmail.com> writes:
> > On Wed, Aug 20, 2025 at 1:03 PM Richard Sandiford
> > <richard.sandif...@arm.com> wrote:
> >>
> >> While testing a later patch, I found that create_degenerate_phi
> >> had an inverted test for bitmap_set_bit. It was assuming that
> >> the return value was the previous bit value, rather than a
> >> "something changed" value. :(
> >>
> >> Also, the call to add_live_out_use shouldn't be conditional
> >> on the DF_LR_OUT operation, since the register could be live-out
> >> because of uses later in the same EBB (which do not require a
> >> live-out use to be added to the rtl-ssa instruction). Instead,
> >> add_live_out should itself check whether a live-out use already exists.
> >>
> >> Tested on aarch64-linux-gnu, powerpc64le-linux-gnu and
> >> x86_64-linux-gnu. OK to install?
> >
> > OK. Is the inverted test worth backporting?
>
> I was wondering that as well. I'm a bit nervous about backporting
> just the removal of !, since without the other part of the fix,
> it could just be substituting one problem for another. But the
> combination of this patch and the find_use patch might be worth
> backporting if there is no fallout.
>
> So how about backporting both patches to GCC 14 and GCC 15 if there
> is no fallout in the next couple of weeks?
That works for me (even just GCC 15).
> I'm reluctant to change
> GCC 13 without a motivating testcase.
Yeah.
Richard.
> Thanks,
> Richard
>
> >
> >> Richard
> >>
> >>
> >> gcc/
> >> * rtl-ssa/blocks.cc (function_info::create_degenerate_phi): Fix
> >> inverted test of bitmap_set_bit. Call add_live_out_use even
> >> if the register was previously live-out from the predecessor block.
> >> Instead...
> >> (function_info::add_live_out_use): ...check here whether a live-out
> >> use already exists.
> >> ---
> >> gcc/rtl-ssa/blocks.cc | 11 +++++------
> >> 1 file changed, 5 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/gcc/rtl-ssa/blocks.cc b/gcc/rtl-ssa/blocks.cc
> >> index 953fd9e516e..a57b9e15f13 100644
> >> --- a/gcc/rtl-ssa/blocks.cc
> >> +++ b/gcc/rtl-ssa/blocks.cc
> >> @@ -315,15 +315,14 @@ function_info::add_live_out_use (bb_info *bb,
> >> set_info *def)
> >>
> >> // If the end of the block already has an artificial use, that use
> >> // acts to make DEF live at the appropriate point.
> >> - use_info *use = def->last_nondebug_insn_use ();
> >> - if (use && use->insn () == bb->end_insn ())
> >> + if (find_use (def, bb->end_insn ()).matching_use ())
> >> return;
> >>
> >> // Currently there is no need to maintain a backward link from the end
> >> // instruction to the list of live-out uses. Such a list would be
> >> // expensive to update if it was represented using the usual insn_info
> >> // access arrays.
> >> - use = allocate<use_info> (bb->end_insn (), def->resource (), def);
> >> + auto *use = allocate<use_info> (bb->end_insn (), def->resource (), def);
> >> use->set_is_live_out_use (true);
> >> add_use (use);
> >> }
> >> @@ -540,12 +539,12 @@ function_info::create_degenerate_phi (ebb_info *ebb,
> >> set_info *def)
> >> basic_block pred_cfg_bb = single_pred (ebb->first_bb ()->cfg_bb ());
> >> bb_info *pred_bb = this->bb (pred_cfg_bb);
> >>
> >> - if (!bitmap_set_bit (DF_LR_IN (ebb->first_bb ()->cfg_bb ()), regno))
> >> + if (bitmap_set_bit (DF_LR_IN (ebb->first_bb ()->cfg_bb ()), regno))
> >> {
> >> // The register was not previously live on entry to EBB and
> >> // might not have been live on exit from PRED_BB either.
> >> - if (bitmap_set_bit (DF_LR_OUT (pred_cfg_bb), regno))
> >> - add_live_out_use (pred_bb, def);
> >> + bitmap_set_bit (DF_LR_OUT (pred_cfg_bb), regno);
> >> + add_live_out_use (pred_bb, def);
> >> }
> >> else
> >> {
> >> --
> >> 2.43.0
> >>
