Hi Richard Thanks a lot for your advises and detailed comments.
We will discuss which instructions need to be accurately classified, and estimate the workload. Regards Qian > -----Original Message----- > From: Richard Sandiford <richard.sandif...@arm.com> > Sent: Tuesday, September 8, 2020 4:21 AM > To: Qian, Jianhua/钱 建华 <qia...@cn.fujitsu.com> > Cc: gcc@gcc.gnu.org > Subject: Re: A problem with one instruction multiple latencies and pipelines > > "Qian, Jianhua" <qia...@cn.fujitsu.com> writes: > > Hi > > > > I'm adding a new machine model. I have a problem when writing the > "define_insn_reservation" for instruction scheduling. > > How to write the "define_insn_reservation" for one instruction that there > > are > different latencies and pipelines according to parameter. > > > > For example, the ADD (shifted register) instruction in a64fx > > > > Instruction Option Latency > Pipeline > > ADD (shifted register) <amount> = 0 1 EX* > | EAG* > > <amount> = [1-4] && <shift>=LSL 1+1 > (EXA + EXA) | (EXB + EXB) > > 2+1 (EXA > + EXA) | (EXB + EXB) > > > > In aarch64.md ADD (shifted register) instruction is defined as following． > > (define_insn "*add_<shift>_<mode>" > > [(set (match_operand:GPI 0 "register_operand" "=r") > > (plus:GPI (ASHIFT:GPI (match_operand:GPI 1 "register_operand" > "r") > > (match_operand:QI 2 > "aarch64_shift_imm_<mode>" "n")) > > (match_operand:GPI 3 "register_operand" "r")))] > > "" > > "add\\t%<w>0, %<w>3, %<w>1, <shift> %2" > > [(set_attr "type" "alu_shift_imm")] > > ) > > > > It could not be distinguished by the type "alu_shift_imm" when writing > "define_insn_reservation" for ADD (shifted register). > > What should I do? > > This is just personal opinion, but in general (from the point of view of a > new port, > or a new subport like SVE), I think the best approach to handling the "type" > attribute is to start with the coarsest classification that makes sense, then > split > these relatively coarse types up whenever there's a specific need. > > When taking that approach, it's OK (and perhaps even a good sign) for an > existing type to sometimes be too coarse for a new CPU. > > So thanks for asking about this, and please don't feel constrained by the > existing "type" classification. IMO we should split existing types wherever > that makes sense for new CPUs. > > In a situation like this, I guess it would make sense to treat alu_shift_imm > as a > conservative general classification and add > alu_shift_imm1to4 (say) as a new type. > > I'd recommend a two-stage approach, with each stage as a separate patch for > ease of review: > > (1) Add the new type to the definition of the "type" attribute and run: > > sed -i s/alu_shift_imm/alu_shift_imm1to4,alu_shift_imm/g > > on all the config/arm and config/aarch64 tuning descriptions. > > This is a purely mechanical replacement and could be tested in > the normal way (i.e. there would be no need to do special testing > to make sure that existing descriptions are unaffected: the change > is mechanical enough not to need that). > > (2) Make the define_insns use the new type where appropriate. > Like Richard says, this would be handled by splitting the single > define_insn alternative above into two separate alternatives. > It's also possible to something like: > > (set (attr "type") > (if_then_else (match_operand ...) > (const_string "...") > (const_string "..."))) > > which localises the change to the attribute definition. > > If we end up needing that construct several times, it would also > be possible to add a new attribute, e.g.: > > (define_attr "shift_imm_type" "none,alu_shift_op2") > > and then change the default value of the type attribute from: > > (const_string "untyped") > > to: > > (cond [(eq_attr "shift_imm_type" "alu_shift_op2") > (if_then_else (match_operand 2 ...) > (const_string "...") > (const_string "..."))] > (const_string "untyped")) > > define_insns like the above could then use: > > (set_attr "shift_imm_type" "alu_shift_op2") > > instead of setting "type" directly. That's definitely more > complicated, but it can help to reduce cut-&-paste. > > Thanks, > Richard >
