On Sat, Feb 23, 2019 at 01:28:22PM +0000, wuyuan (E) wrote: > Hi ,James: > Sorry for not responding to your email in time because of Chinese New Year’s > holiday and urgent work. The three questions you mentioned last email are due > to my misunderstanding of pipeline. > the first question, These instructions will occupy both the tsv110_ls* and > tsv110_fsu* Pipeline at the same time.
Hi Wuyuan, Please accept my apologies for how long it has taken me to revisit your patch and review it. I have two questions: > +(define_insn_reservation "tsv110_crypto_sha256_fast" 2 > + (and (eq_attr "tune" "tsv110") > + (eq_attr "type" "crypto_sha1_fast")) > + "tsv110_fsu1") I think you intended to check for type crypto_sha256_fast here. > +;; ALU ops with shift > +(define_insn_reservation "tsv110_alu_shift" 2 > + (and (eq_attr "tune" "tsv110") > + (eq_attr "type" "extend,\ > + alu_shift_imm,alu_shift_reg,\ > + crc,logic_shift_imm,logic_shift_reg,\ > + mov_shift,mvn_shift,\ > + mov_shift_reg,mvn_shift_reg")) > + "tsv110_mdu") > + > +(define_insn_reservation "tsv110_alus_shift" 2 > + (and (eq_attr "tune" "tsv110") > + (eq_attr "type" "alus_shift_imm,alus_shift_reg,\ > + logics_shift_imm,logics_shift_reg")) > + "tsv110_alu2") Is this the correct description? This code says that ALU operations with shift are executed in MDU, but ALU operations with shift that are also flag setting are executed in ALU2? Otherwise, this patch is OK for trunk. Thank you for your patience. Best Regards, James > rewritten as follows: > (define_insn_reservation > "tsv110_neon_ld4_lane" 9 > (and (eq_attr "tune" "tsv110") > (eq_attr "type" "neon_load4_all_lanes,neon_load4_all_lanes_q,\ > neon_load4_one_lane,neon_load4_one_lane_q")) > "(tsv110_ls1 + tsv110_fsu1)|(tsv110_ls1 + tsv110_fsu2)|(tsv110_ls2 + > tsv110_fsu1)|(tsv110_ls2 + tsv110_fsu2)") > > the second question, These instructions will use tsv110_fsu1 Pipeline or > tsv110_fsu2 Pipeline. > rewritten as follows: > (define_insn_reservation "tsv110_neon_abd_aba" 4 > (and (eq_attr "tune" "tsv110") > (eq_attr "type" "neon_abd,neon_arith_acc")) > "tsv110_fsu1|tsv110_fsu2") > > the third question, These instructions will use tsv110_fsu1 Pipeline or > tsv110_fsu2 Pipeline. > rewritten as follows: > (define_insn_reservation "tsv110_neon_abd_aba_q" 4 > (and (eq_attr "tune" "tsv110") > (eq_attr "type" "neon_arith_acc_q")) > "tsv110_fsu1|tsv110_fsu2") > > In addition to the above changes, I asked hardware engineers and colleagues > to review my patch and modify some of the errors. The detailed patches are > as follows: > > * config/aarch64/aarch64-cores.def (tsv1100): Change scheduling model. > * config/aarch64/aarch64.md : Add "tsv110.md" > * config/aarch64/tsv110.md: New file. >
