On Wed, Jun 2, 2021 at 12:01 PM Kewen.Lin <li...@linux.ibm.com> wrote: > > on 2021/6/2 下午5:13, Richard Sandiford wrote: > > "Kewen.Lin" <li...@linux.ibm.com> writes: > >> Hi Richard, > >> > >> on 2021/6/2 锟斤拷锟斤拷4:11, Richard Sandiford wrote: > >>> Kewen Lin <li...@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).
So what Richard suggests would be to disallow split conditions that do not start with "&& ", it's probably easy to do that as well and look for build fails. That should catch all cases to look at. Richard. > BR, > Kewen >
