On 2012/7/16 12:28 AM, Richard Sandiford wrote: > Chung-Lin Tang <clt...@codesourcery.com> writes: >> This patch adds scheduling support for the NetLogic XLP, including a new >> pipeline description, and associated changes. >> >> Asides from the new xlp.md description file, there are also some sync >> primitive attribute modifications, for better scheduling of sync loops >> (Maxim should be able to better explain this). > > Rather than add a "type" attribute to each sync loop, please just add: > > (not (eq_attr "sync_mem" "none")) > (symbol_ref "syncloop") > > to the default value of the "type" attribute. You'll probably need > to swap the order of the sync* attributes with the "type" attribute > in order for this to compile. > > The patch is effectively changing the type of the sync loops from > "unknown" to "syncloop". That's certainly OK, but you'll need to > add "syncloop" to the "unknown" reservations of all other schedulers > (except for generic.md, where what you've done instead is fine). > It might be easier if you split out the addition of syncloop > as a separate patch.
I'll leave it to Maxim to respond to the sync parts. >> Other generic changes include a new "hilo" insn attribute, to mark which >> of HI/LO does a m[ft]hilo insn access. > > The way other schedulers handle this is with things like: > > (define_insn_reservation "ir_sb1_mfhi" 1 > (and (eq_attr "cpu" "sb1,sb1a") > (and (eq_attr "type" "mfhilo") > (not (match_operand 1 "lo_operand")))) > "sb1_ex1") > > which seems simpler. mfhilo and mthilo are required to read operand 1 > and write to operand 0 (respectively) in order to support this kind of > construct. > > That said, even the above is a hold-over from when we tried to allow > high registers to store independent values. These days we can be a bit > more precise, as with the patch below. (As the comment says: > > ;; If a doubleword move uses these expensive instructions, > ;; it is usually better to schedule them in the same way > ;; as the singleword form, rather than as "multi". > > I'm continuing to assume that mflo and mtlo are the best type choices > for unsplit double-register moves. That path should be very rarely > outside of MIPS16 anyway -- just by sched1 if hi and lo are exposed > directly -- and no current scheduler tries to model a doubleword hi/lo > move separately from single-register ones. The information is available > via the dword_mode attribute if required.) I suppose this means that actual generation of moves as mfhi/mthi should almost never happen out of normal conditions? > Tested on mips64-elf, and by making sure that there were no changes in > -O2 output for a recent set of cc1 .ii files. Applied. > > I'm probably punishing you for being honest here, but the only other > thing is that you've listed NetLogic Microsystems Inc. as one of the > authors. I think that means they'll need to sign a copyright assignment. > Have they already done that? They have assigned the copyright to Mentor Graphics, so it should mean the code can be contributed by us. Thanks, Chung-Lin
