On 29 March 2011 09:55, Alexander Graf <ag...@suse.de> wrote: > > On 28.03.2011, at 17:40, Peter Maydell wrote: > >> On 24 March 2011 15:58, Alexander Graf <ag...@suse.de> wrote: >>> diff --git a/target-s390x/translate.c b/target-s390x/translate.c >>> + case 0x4: /* LMG R1,R3,D2(B2) [RSE] */ >>> + case 0x24: /* STMG R1,R3,D2(B2) [RSE] */ >>> + case 0x26: /* STMH R1,R3,D2(B2) [RSE] */ >>> + case 0x96: /* LMH R1,R3,D2(B2) [RSE] */ >>> + /* Apparently, unrolling lmg/stmg of any size gains performance - >>> + even for very long ones... */ >> >> Doesn't this take you over MAX_OP_PER_INSTR for some cases? > > I haven't encountered any case where it does.
Really? MAX_OP_PER_INSTR's only 96, so if you have 16 registers in your loop then it only needs 6 ops per register to hit that, and the op 0x96 case looks like it must generate more than that. I have an item on my todo list to see if I can add an assert() check for this limit, because there are cases for Neon load/stores that apparently hit it. >>> + tmp2 = tcg_const_i64((((uint64_t)i2) << 48) | >>> 0x0000ffffffffffffULL); >> >> This line is over 80 chars, as are a handful of others in this file. > > Yeah, I generally see the 80 char limit as soft limit and make it > hard on ~90. If a line is only over it by very little, readability > doesn't improve by breaking it up. So far, everyone agreed to that > approach :). >80 chars reduces readability for me because I have emacs configured to make long lines look very ugly so I don't write them :-) Also, if we want the standard to be 'soft 80, hard 90' we should say so in CODING_STYLE... >>> + case 0xa: /* SVC I [RR] */ >>> + insn = ld_code2(s->pc); >>> + debug_insn(insn); >>> + i = insn & 0xff; >>> +#ifdef CONFIG_USER_ONLY >>> + s->pc += 2; >>> +#endif >>> + update_psw_addr(s); >>> + gen_op_calc_cc(s); >> >> Why do we only need to update s->pc if CONFIG_USER_ONLY? >> Not saying it's wrong, but it could use an explanatory comment... > > The user code needs to know where it jumps back to, while the > exception generation code needs to get the exact position it was > in to generate some more metadata. Ah. For ARM we do this by advancing env->regs[15] in linux-user/main.c cpu_loop() when we get an EXCP_SWI. It looks like we do it that way for MIPS and SPARC at least too, so I guess it would be better for s390 to follow that pattern. -- PMM
