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));
> >
