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

Kewen.Lin linkw@linux.ibm.com
Wed Jun 2 10:01:29 GMT 2021


on 2021/6/2 下午5:13, Richard Sandiford wrote:
> "Kewen.Lin" <linkw@linux.ibm.com> writes:
>> Hi Richard,
>>
>> on 2021/6/2 锟斤拷锟斤拷4:11, Richard Sandiford wrote:
>>> Kewen Lin <linkw@linux.ibm.com> writes:
>>>> Hi all,
>>>>
>>>> define_insn_and_split should avoid to use empty split condition
>>>> if the condition for define_insn isn't empty, otherwise it can
>>>> sometimes result in unexpected consequence, since the split
>>>> will always be done even if the insn condition doesn't hold.
>>>>
>>>> To avoid forgetting to add "&& 1" onto split condition, as
>>>> Segher suggested in thread[1], this series is to add the check
>>>> and raise an error if it catches the unexpected cases.  With
>>>> this new check, we have to fix up some existing
>>>> define_insn_and_split which are detected as error.  I hope all
>>>> these places are not intentional to be kept as blank.
>>>
>>> I wonder whether we should instead redefine the semantics of
>>> define_insn_and_split so that the split condition is always applied
>>> on top of the insn condition.  It's rare for a define_insn_and_split
>>> to have independent insn and split conditions, so at the moment,
>>> we're making the common case hard.
>>>
>>
>> Just want to confirm that the suggestion is just applied for empty
>> split condition or all split conditions in define_insn_and_split? 
>> I guess it's the former?
> 
> No, I meant tha latter.  E.g. in:
> 
> (define_insn_and_split
>   […]
>   "TARGET_FOO"
>   "…"
>   […]
>   "reload_completed"
>   […]
> )
> 
> the "reload_completed" condition is almost always a typo for
> "&& reload_completed".
> 
> Like I say, it rarely makes sense for the split condition to
> ignore the insn condition and specify an entirely independent condition.
> There might be some define_insn_and_splits that do that, but it'd often
> be less confusing to write the insn and split separately if so.
> 
> Even if we do want to support independent insn and split conditions,
> that's always going to be the rare and surprising case, so it's the one
> that should need extra syntax.
> 

Thanks for the clarification!

Since it may impact all ports, I wonder if there is a way to find out
this kind of "rare and surprising" case without a big coverage testing?
I'm happy to make a draft patch for it, but not sure how to early catch
those cases which need to be rewritten for those ports that I can't test
on (even with cfarm machines, the coverage seems still limited).

BR,
Kewen



More information about the Gcc-patches mailing list