On 09/05/17 23:27, Wilco Dijkstra wrote: > Bernd Edlinger wrote: >> No, the split condition does not begin with "&& TARGET_32BIT...". >> Therefore the split is enabled in TARGET_NEON after reload_completed. >> And it is invoked from adddi3_neon for all alternatives without vfp >> registers: > > Hmm that's a huge mess. I'd argue that any inst_and_split should only split > it's own instruction, never other instructions (especially if they are from > different md files, which is extremely confusing). Otherwise we should use > a separate split and explicitly list which instructions it splits. >
This is literally a mine-field. > So then the next question is whether the neon_adddi3 still needs the > arm_adddi3 splitter in some odd corner cases? > Yes, I think so. While *arm_adddi3 and adddi3_neon insn are mutually exclusive, they share the splitter. I don't know with which iwmmxt-pattern the *arm_adddi3-splitter might interfere, therefore I don't know if the !TARGET_IWMMXT can be removed from the splitter condition. Other patterns have a iwmmxt-twin that is not mutually exclusive. For instance *anddi_notdi_di, bicdi3_neon and iwmmxt_nanddi3 The bicdi3_neon insn duplicates the alternatives while iwmmxt does not. And nobody is able to test iwmmxt. >>> Also there are more cases, a quick grep suggests *anddi_notdi_di has the >>> same issue. > >> Yes, that pattern can be cleaned up in a follow-up patch. > > And there are a lot more instructions that need the same treatment and split > early (probably best at expand time). I noticed none of the zero/sign extends > split before regalloc for example. > I did use the test cases as benchmarks to decide which way to go. It is quite easy to try different combinations of cpu and inspect the stack usage of neon, iwmmxt, and vfp etc. It may be due to interaction with other patterns, but not in every case the split at expand time produced the best results. >> Note this splitter is invoked from bicdi3_neon as well. >> However I think anddi_notdi_di should be safe as long as it is enabled >> after reload_completed (which is probably a bug). > > Since we should be splitting and/bic early now I don't think you can get > anddi_notdi > anymore. So it could be removed completely assuming Neon already does the > right > thing. > > It looks like we need to do a full pass over all DI mode instructions and clean up > all the mess. > Yes, but in small steps, and using some benchmarks to make sure that each step does improve at least something. Bernd. > Wilco >
