On 29.03.2011, at 11:17, Peter Maydell wrote: > 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.
Hrm - might be useful to increase MAX_OP_PER_INSTR then, no? > >>>> + 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 :-) Heh, I have vi configured to color in lines >80 chars as well, so I usually only keep them there > Also, if we want the standard to be 'soft 80, hard 90' we should > say so in CODING_STYLE... *shrug* so far CODING_STYLE has only brought badness to qemu. The new style is less readable than the common dominator that was there before (Fabrice's coding style) and resulted in man-years of wasted time on rejected patches for the sake of braces. I'd rather want to remove the file than patching it (which again would create a mail thread of 300 mails, waste 5 man-years of productivity and bring us no gain in the end). > >>>> + 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. Unfortunately, it's not that easy as there are 2 different ways of issuing an SVC (actual SVC and EXECUTE instruction), both of which having different instruction lengths. So we really need to keep the information in the instruction decoder :( Alex