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

[Segher Boessenkool]

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 :-)
> 
> > > 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.


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