on 2021/6/3 下午4:05, Richard Sandiford wrote: > "Kewen.Lin" <li...@linux.ibm.com> writes: >> Hi Richi/Richard/Jeff/Segher, >> >> Thanks for the comments! >> >> on 2021/6/3 锟斤拷锟斤拷7:52, Segher Boessenkool wrote: >>> On Wed, Jun 02, 2021 at 06:32:13PM +0100, Richard Sandiford wrote: >>>> Richard Biener <richard.guent...@gmail.com> writes: >>>>> 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. >>>> >>>> Yeah. As a strawman proposal, how about: >>>> >>>> - add a new "define_independent_insn_and_split" that has the >>>> current semantics of define_insn_and_split. This should be >>>> mechanical. >>> >>> I'd rather not have that -- we can just write separate define_insn and >>> define_split in that case. >>> >> >> Not sure if someone would argue that he/she would like to go with one shared >> pattern as before, to avoid any possible differences between two seperated >> patterns and have good maintainability (like only editing on place) and >> slightly better efficiency. > > Right. Plus it creates less make-work. If we didn't have it, someone > would need to split the define_insn_and_splits that don't currently > use "&&", then later someone might decide that the missing "&&" was a > mistake and need to put them together again (or just revert the patch > and edit from there, I guess). > > Plus define_independent_insn_and_split would act as a flag for something > that might be suspect. If we split them then the define_split condition > will seem to have been chosen deliberately in isolation. > >>> How many such cases *are* there? There are no users exposed to this, >>> and when the split condition is required to start with "&&" (instead of >>> getting that implied) it is not a silent change ever, either. >>> >> >> If I read the proposal right, the explicit "&&" is only required when going >> to find all potential problematic places for final implied "&&" change. >> But one explicit "&&" does offer good readability. > > I don't know. "&& 1" looks kind of weird to me. > > One thing I'd been wondering about a while ago was whether we should key > the split part of define_insn_and_splits off the insn code, instead of > repeating the pattern match and insn C condition. That would make the > split apply only to the associated define_insns, whereas at the moment > they also apply to any earlier (less general) define_insn that happens > to match the pattern and the C conditions. It would also reduce the > complexity of the autogenerated define_split logic. > > I don't know whether that's a good idea or not. But having an explicit > "&&" implies that the generator shouldn't do that, and that it should > retest the insn condition from scratch. > >>>> - find the define_insn_and_splits that are missing the "&&", and where >>>> missing the "&&" might make a difference. Change them to >>>> define_independent_insn_and_splits. >>>> >>>> Like Richard says, this can be done by temporarily disallowing >>>> define_insn_and_splits that have no "&&". >>> >>> If we make that change permanently, that is all steps we ever need! >>> >> >> So the question is that: whether we need to demand an explicit "&&". >> Richard's proposal is for answer "no" which aligns with Richi's auto >> filling advice before. I think it would result in fewer changes since >> those places without explicit "&&" are mostly unintentional, all the jobs >> are done by implied "&&". Its downside seems to be bad readability, new >> readers may take it as two seperated conditions at first glance, but I >> guess if we emphasize this change in the document it would be fine? >> Or emitting one warning if missing an explicit "&&"? > > IMO the natural way to read it is that the split C condition gives the > conditions under which the instruction should be split. I think that's > why forgetting the "&&" is such a common mistake. (I know I've done it > plenty of times.) > > IMO requiring the "&&" is baking in an alternative, less intuitive, > interpretation. >
Thanks for the explanation, I was thinking people may have got used to the starting "&&" in split condition, so it's easy for them to read. But I agree it's better not to have it in the natural way. BR, Kewen
