"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, Richard
