On Wednesday 17 January 2018 07:07 PM, Wilco Dijkstra wrote: > (finished version this time, somehow Outlook loves to send emails early...) > > Hi, > > In general I think the best way to achieve this would be to use the > existing cost models which are there for exactly this purpose. If > this doesn't work well enough then we should fix those. As is, > this patch disables a whole class of instructions for a specific > target rather than simply telling GCC that they are expensive and > should only be used if there is no cheaper alternative.
The current cost model will disable reg offset for loads as well as stores, which doesn't work well since loads with reg offset are faster for falkor. Also, this is a very specific tweak for a specific processor, i.e. I don't know if there is value in splitting out the costs into loads and stores and further into 128-bit and lower just to set the 128 store cost higher. That will increase the size of the change by quite a bit and may not make it suitable for inclusion into gcc8 at this stage, while the current one still qualifies given its contained impact. Further, it seems like worthwhile work only if there are other parts that actually have the same quirk and can use this split. Do you know of any such cores? > Also there is potential impact on generic code from: > > (define_insn "*aarch64_simd_mov<VQ:mode>" > [(set (match_operand:VQ 0 "nonimmediate_operand" > - "=w, Umq, m, w, ?r, ?w, ?r, w") > + "=w, Umq, Utf, w, ?r, ?w, ?r, w") > (match_operand:VQ 1 "general_operand" > - "m, Dz, w, w, w, r, r, Dn"))] > + "m, Dz, w, w, w, r, r, Dn"))] > > It seems an 'm' constraint has special meaning in the register allocator, > using a different constraint can block certain simplifications (for example > merging stack offsets into load/store in the post-reload cleanup pass), > so we'd need to verify this doesn't cause regressions. I'll verify this. > Also it is best to introduce generic interfaces: > > +/* Return TRUE if OP is a good address mode for movti target on falkor. */ > +bool > +aarch64_falkor_movti_target_operand_p (rtx op) > > +(define_memory_constraint "Utf" > + "@iternal > + A good address for a falkor movti target operand." > + (and (match_code "mem") > + (match_test "aarch64_falkor_movti_target_operand_p (op)"))) > > We should use generic names here even if the current implementation > wants to do something specific for Falkor. I'll fix this. Thanks, Siddhesh
