on 2021/6/2 下午3:43, Richard Biener wrote:
> 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.
Yes, the "" in split condition does mean 'true' (always).
> 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?
Without this patch, they are indeed functionally identical. It's like
the writer want to have one define_insn to match under some condition, but
want to have one define_split to match always.
> Also "" as split condition _does_
> seem valid, just maybe unintended?
Yes, it's valid without this patch. That's why I asked whether there is
some good reason to keep it be [1]. In Segher's opinion, there is no
good reason, he pointed out "A reader does not expect a
define_insn_and_split to split any other insns."
> How would one create a
> functionally equivalent example? "|| 1" will likely not work.
>
I think "|| 1" works just like "" if people want the define_split to
split all the time, even with this patch.
> 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.
>
Yes, the current patch will stop the "" condition which was accepted
before. Thanks for bringing this up! We have to update the
documentation if people reach a consensus.
[1] https://gcc.gnu.org/pipermail/gcc-patches/2021-March/567014.html
BR,
Kewen
