From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NYNof-0005le-5y for qemu-devel@nongnu.org; Fri, 22 Jan 2010 13:01:05 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NYNod-0005l9-VM for qemu-devel@nongnu.org; Fri, 22 Jan 2010 13:01:04 -0500 Received: from [199.232.76.173] (port=54343 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NYNod-0005l4-Mc for qemu-devel@nongnu.org; Fri, 22 Jan 2010 13:01:03 -0500 Received: from are.twiddle.net ([75.149.56.221]:35654) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NYNoc-0001u5-Ul for qemu-devel@nongnu.org; Fri, 22 Jan 2010 13:01:03 -0500 Message-ID: <4B59E7D9.5050908@twiddle.net> Date: Fri, 22 Jan 2010 10:00:57 -0800 From: Richard Henderson MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] Porting TCG to alpha platform References: <242393.28161.qm@web15901.mail.cnb.yahoo.com> In-Reply-To: <242393.28161.qm@web15901.mail.cnb.yahoo.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: identifier scorpio Cc: qemu-devel@nongnu.org On 01/22/2010 07:47 AM, identifier scorpio wrote: > Is there any good method to find the bug except "guess and try"? -singlestep -d in_asm,cpu,exec Use these options on both Alpha host and x86_64 host and diff the resulting output files. That will show you which target instruction is emulated differently. > And it seems that few people is interested in porting QEMU/TCG to > alpha platform, why? just because alpha machine is disappearing? Probably. I no longer have functioning alpha hardware, which is why I've become interested in qemu with alpha as a target. > + tcg_out32(s, (opc)|INSN_RA(ra)|INSN_DISP21(disp)); Coding style: spaces around operators, no useless parens around opc. > +static inline int tcg_target_const_match( \ > + tcg_target_long val, const TCGArgConstraint *arg_ct) Coding style: no trailing \; there's no need for line continuation anywhere except in preprocessor directives. Merge the first argument into the first line, assuming that can happen without exceeding 80 columns. Arguments that do get split to subsequent lines should be properly indented below the first argument on the previous line. > + if ( type == TCG_TYPE_I32) > + val = (int32_t)val; Coding style: use braces, even for a single statement. Yes, there's plenty of code in the project that doesn't, but the coding style is being enforced on all new code. Also, extra space after open paren. > + if (disp != (int16_t)disp) { > + tcg_out_movi(s, TCG_TYPE_I64, TMP_REG1, disp); > + tcg_out_fmt_opr(s, INSN_ADDQ, rb, TMP_REG1, TMP_REG1); > + tcg_out_fmt_mem(s, opc, ra, TMP_REG1, 0); > + } else { > + tcg_out_fmt_mem(s, opc, ra, rb, disp); > + } The reason I suggested writing the tcg_out_opc_long as I did was so that for a 32-bit displacement like 0x12345678 instead of ldah $28,0x1234($31) lda $28,0x5678($28) addq $16,$28,$28 ldq $17,0($28) we can generate ldah $28,0x1234($16) ldq $17,0x5678($28) For larger 64-bit constants obviously we have no choice but to use an ADDQ, but even then one can stuff the final 16-bit addition into the memory insn itself. Given the size of the CPUState, offsets > 32k are very common, so it would be good to handle this case well. > +static void tcg_out_mov_addr( TCGContext *s, int ret, int addr) > +{ > + tcg_out_mov(s, ret, addr); > +#if TARGET_LONG_BITS == 32 > + /* if VM is of 32-bit arch, clear higher 32-bit of addr */ > + tcg_out_fmt_opi(s, INSN_ZAPNOT, ret, 0x0f, ret); > +#endif > +} It's a bit wasteful to emit the move *and* the zap; give "addr" to the first operand of the zapnot directly. > + tgen_arithi(s, INSN_ADDQ, r1, offsetof(CPUState, tlb_table[mem_index][0].addr_read)); > + tcg_out_fmt_opr(s, INSN_ADDQ, r1, TCG_REG_15, r1); > +#if TARGET_LONG_BITS == 32 > + tcg_out_fmt_mem(s, INSN_LDL, TMP_REG1, r1, 0); > + tcg_out_fmt_opi(s, INSN_ZAPNOT, TMP_REG1, 0x0f, TMP_REG1); > +#else > + tcg_out_fmt_mem(s, INSN_LDQ, TMP_REG1, r1, 0); > +#endif Better as tcg_out_fmt_opr(s, INSN_ADDQ, r1, TCG_REG_15, r1); tcg_out_ldst(s, TARGET_LONG_BITS == 32 ? INSN_LDL : INSN_LDQ, TMP_REG1, r1, offsetof(CPUState, ...), 0); to fold part of the offset into the memory displacement, and reuse your existing code emitting the zapnot. You might also consider copying the ppc64 port and split out a tcg_out_tlb_read function from both qemu_ld and qemu_st. > + *(uint32_t *)label1_ptr = (uint32_t) \ > + ( INSN_BNE | ( TMP_REG1 << 21 ) | ( val & 0x1fffff)); If you're not going to use the gen_new_label method, at least use your INSN_RA and INSN_DISP21 macros. > + tcg_out_movi(s, TCG_TYPE_I64, TMP_REG1, \ > + offsetof(CPUTLBEntry, addend) - offsetof(CPUTLBEntry, addr_read)); > + tcg_out_fmt_opr(s, INSN_ADDQ, r1, TMP_REG1, r1); > + tcg_out_fmt_mem(s, INSN_LDQ, TMP_REG1, r1, 0); As above, better to use tcg_out_ldst. > + { INDEX_op_add_i32, { "r", "0", "r" } }, All of these matching constraints are wrong -- alpha is a 3 address machine; should be { "r", "r", "r" }, at least until you support constant arguments properly. I still haven't seen anything that looks actively wrong to explain why windows isn't working for you... r~