[PATCH 01/11] gen: Emit error msg for empty split condition

Richard Biener richard.guenther@gmail.com
Wed Jun 2 07:43:22 GMT 2021


On Wed, Jun 2, 2021 at 9:28 AM Kewen.Lin <linkw@linux.ibm.com> wrote:
>
> Hi Richi,
>
> on 2021/6/2 下午3:04, Richard Biener wrote:
> > On Wed, Jun 2, 2021 at 7:05 AM Kewen Lin <linkw@linux.ibm.com> wrote:
> >>
> >> As Segher suggested, this patch is to emit the error message
> >> if the split condition of define_insn_and_split is empty while
> >> the insn condition isn't.
> >
> > I wonder whether it would be a good idea to automagically make
> > the split condition "&& 1" via gensupport?
> >
>
> Thanks for the comment!  Do you happen to have some similar examples?

Not sure, the docs say

@var{insn-pattern}, @var{condition}, @var{output-template}, and
@var{insn-attributes} are used as in @code{define_insn}.
...
The @var{split-condition} is also used as in
@code{define_split}, with the additional behavior that if the condition starts
with @samp{&&}, the condition used for the split will be the constructed as a
logical ``and'' of the split condition with the insn condition.

so one can indeed read this as "" meaning 'true' w/o considering the
define_insn condition.  But then we say

The @code{define_insn_and_split} construction provides exactly the same
functionality as two separate @code{define_insn} and @code{define_split}
patterns.  It exists for compactness, and as a maintenance tool to prevent
having to ensure the two patterns' templates match.

But then when I split a define_insn_and_split with a "" split condition
they are not functionally identical?  Also "" as split condition _does_
seem valid, just maybe unintended?  How would one create a
functionally equivalent example? "|| 1" will likely not work.

Note I'm not familiar with all the details here but the documentation
does seem ambiguous at best, not supporting to error on empty
split-conditions at least.

Richard.

> IIUC this idea has to lay on the assumption always holding that when
> one developer puts split condition as blank meanwhile leaves insn
> condition as non-empty, he/she exactly wants to make split condition
> the same as insn condition.  Once one developer doesn't think like
> that way (that is to want split to deal with more), the automatic
> condtion filling seems to over fill.
>
> The way that the current patch provided is not to allow the developer
> to write that kind of pattern, instead he/she has to go with separated
> define_insn and define_split.
>
> BR,
> Kewen
>
> >> gcc/ChangeLog:
> >>
> >>         * gensupport.c (process_rtx): Emit error message for empty
> >>         split condition in define_insn_and_split while the insn
> >>         condition isn't.
> >> ---
> >>  gcc/gensupport.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/gcc/gensupport.c b/gcc/gensupport.c
> >> index 0f19bd70664..52cee120215 100644
> >> --- a/gcc/gensupport.c
> >> +++ b/gcc/gensupport.c
> >> @@ -620,6 +620,9 @@ process_rtx (rtx desc, file_location loc)
> >>           }
> >>         else if (GET_CODE (desc) == DEFINE_INSN_AND_REWRITE)
> >>           error_at (loc, "the rewrite condition must start with `&&'");
> >> +       else if (split_cond[0] == '\0' && strlen (XSTR (desc, 2)) != 0)
> >> +         error_at (loc, "the split condition mustn't be empty if the "
> >> +                        "insn condition isn't empty");
> >>         XSTR (split, 1) = split_cond;
> >>         if (GET_CODE (desc) == DEFINE_INSN_AND_REWRITE)
> >>           XVEC (split, 2) = gen_rewrite_sequence (XVEC (desc, 1));
> >> --
> >> 2.17.1
> >>
>


More information about the Gcc-patches mailing list