[RFC/PATCH 00/11] Fix up some unexpected empty split conditions

Richard Biener richard.guenther@gmail.com
Mon Jun 7 07:12:22 GMT 2021


On Fri, Jun 4, 2021 at 4:58 AM Kewen.Lin via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Hi Segher,
>
> on 2021/6/3 下午5:18, Segher Boessenkool wrote:
> > On Thu, Jun 03, 2021 at 03:00:44AM -0500, Segher Boessenkool wrote:
> >> On Thu, Jun 03, 2021 at 01:22:38PM +0800, Kewen.Lin wrote:
> >> The whole point of requiring the split condition to start with && is so
> >> it will become harder to mess things up (it will make the gen* code a
> >> tiny little bit simpler as well).  And there is no transition period or
> >> anything like that needed either.  Just the bunch that will break will
> >> need fixing.  So let's find out how many of those there are :-)
> >>
>
> To find out those need fixing seems to be the critical part.  It's
> not hard to add one explicit "&&" to those that don't have it now, but
> even with further bootstrapped and regression tested I'm still not
> confident the adjustments are safe enough, since the testing coverage
> could be limited.  It may need more efforts to revisit, or/and test
> with more coverages, and port maintainers' reviews.
>
> In order to find one example which needs more fixing, for rs6000/i386/
> aarch64, I fixed all define_insn_and_splits whose insn cond isn't empty
> (from gensupport's view since the iterator can add more on) while split
> cond don't start with "&&" , also skipped those whose insn conds are
> the same as their split conds.  Unfortunately (or fortunately :-\) all
> were bootstrapped and regress-tested.
>
> The related diffs are attached, which is based on r12-0.

For preserving existing behavior with requiring "&& " we can (mechanically?)
split define_insn_and_split into define_insn and define_split with the old
condition, right?

That is, all empty insn condition cases are of course obvious to fix.

> >>>> How many such cases *are* there?  There are no users exposed to this,
> >>>> and when the split condition is required to start with "&&" (instead of
> >>>> getting that implied) it is not a silent change ever, either.
> >>>
> >>> If I read the proposal right, the explicit "&&" is only required when going
> >>> to find all potential problematic places for final implied "&&" change.
> >>> But one explicit "&&" does offer good readability.
> >>
> >> My proposal is to always require && (or maybe identical insn and split
> >> conditions should be allowed as well, if people still use that -- that
> >> is how we wrote "&& 1" before that existed).
> >
> > I prototyped this.  There are very many errors.  Iterators often modify
> > the insn condition (for one iteration of it), but that does not work if
> > the split condition does not start with "&&"!
> >
> > See attached prototype.
> >
> >
>
> Thanks for the prototype!
>
> BR,
> Kewen
>
> > Segher
> >
> > = = =
> >
> > diff --git a/gcc/gensupport.c b/gcc/gensupport.c
> > index 2cb760ffb90f..05d46fd3775c 100644
> > --- a/gcc/gensupport.c
> > +++ b/gcc/gensupport.c
> > @@ -590,7 +590,6 @@ process_rtx (rtx desc, file_location loc)
> >      case DEFINE_INSN_AND_SPLIT:
> >      case DEFINE_INSN_AND_REWRITE:
> >        {
> > -     const char *split_cond;
> >       rtx split;
> >       rtvec attr;
> >       int i;
> > @@ -611,15 +610,20 @@ process_rtx (rtx desc, file_location loc)
> >
> >       /* If the split condition starts with "&&", append it to the
> >          insn condition to create the new split condition.  */
> > -     split_cond = XSTR (desc, 4);
> > -     if (split_cond[0] == '&' && split_cond[1] == '&')
> > +     const char *insn_cond = XSTR (desc, 2);
> > +     const char *split_cond = XSTR (desc, 4);
> > +     if (!strncmp (split_cond, "&&", 2))
> >         {
> >           rtx_reader_ptr->copy_md_ptr_loc (split_cond + 2, split_cond);
> > -         split_cond = rtx_reader_ptr->join_c_conditions (XSTR (desc, 2),
> > +         split_cond = rtx_reader_ptr->join_c_conditions (insn_cond,
> >                                                           split_cond + 2);
> > +       } else if (insn_cond[0]) {
> > +         if (GET_CODE (desc) == DEFINE_INSN_AND_REWRITE)
> > +           error_at (loc, "the rewrite condition must start with `&&'");
> > +         else
> > +           error_at (loc, "the split condition must start with `&&' [%s]", insn_cond);
> >         }
> > -     else if (GET_CODE (desc) == DEFINE_INSN_AND_REWRITE)
> > -       error_at (loc, "the rewrite condition must start with `&&'");
> > +
> >       XSTR (split, 1) = split_cond;
> >       if (GET_CODE (desc) == DEFINE_INSN_AND_REWRITE)
> >         XVEC (split, 2) = gen_rewrite_sequence (XVEC (desc, 1));
> >


More information about the Gcc-patches mailing list