On Wed, Jun 2, 2021 at 9:28 AM Kewen.Lin <li...@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 <li...@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
> >>
>
