[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 03/15] Openrisc: add instructions translation
From: |
Max Filippov |
Subject: |
Re: [Qemu-devel] [PATCH 03/15] Openrisc: add instructions translation |
Date: |
Fri, 18 May 2012 14:33:13 +0400 |
Hi Jia.
>>> + case 0x0009:
>>> + switch (op1) {
>>> + case 0x03: /*l.div*/
>>> + LOG_DIS("l.div r%d, r%d, r%d\n", rd, ra, rb);
>>> + if (rb != 0) {
>>> + tcg_gen_div_tl(cpu_R[rd], cpu_R[ra], cpu_R[rb]);
>>
>> You also need to take care of integer overflow/division by zero here,
>> otherwise it may crash QEMU.
>>
>>> + } else {
>>> + /* exception here */
>
> overflow/division by zero is handled here by patch 05/15.
In that patch I have found only exception generation in the 'else' branch.
However code generated by tcg_gen_div_tl in the 'if' branch will crash QEMU
in case rb register carries zero or ra/rb gives integer overflow.
>>> + case 0x000b:
>>> + switch (op1) {
>>> + case 0x03: /*l.mulu*/
>>> + LOG_DIS("l.mulu r%d, r%d, r%d\n", rd, ra, rb);
>>> + if (rb != 0 && ra != 0) {
>>> + tcg_gen_ext32u_tl(cpu_R[ra], cpu_R[ra]);
>>> + tcg_gen_ext32u_tl(cpu_R[rb], cpu_R[rb]);
>>
>> This code would clobber high 32 bits of ra and rb if it was compiled
>> for 64 bit registers.
>> Are you going to support both 32 and 64 bit version of the ISA?
>> Could you please clarify *_tl usage here?
>>
>
> I used uint32_t at first, change it into *_tl for 64bit support in the future.
> May I keep it *_tl? If I can not, I'll back it to uint32.
Ok, to me *_tl looks pretty reasonable for instructions that scale uniformly
with register width, like simple l.and/l.or/l.add/l.div/etc.
For instructions that has less symmetry regarding register width or that
operate on registers of different widths at once, like l.maci, I'd say that
explicit *_i32/*_i64 would be more informative.
For this particular mulu I guess the correct implementation, suitable for both
32 and 64 bit wide registers would be just
tcg_gen_mul_tl(cpu_R[rd], cpu_R[ra], cpu_R[rb]);
however it doesn't take care of SR[OV], SR[CY]. I haven't found a place
where you update these bits.
>>> +static void dec_float(DisasContext *dc, CPUOPENRISCState *env, uint32_t
>>> insn)
>>> +{
>>> + uint32_t op0;
>>> + uint32_t ra, rb, rd;
>>> + op0 = field(insn, 0, 8);
>>> + ra = field(insn, 16, 5);
>>> + rb = field(insn, 11, 5);
>>> + rd = field(insn, 21, 5);
>>> +
>>> + switch (op0) {
>>> + case 0x10: /*lf.add.d*/
>>> + LOG_DIS("lf.add.d r%d, r%d, r%d\n", rd, ra, rb);
>>> + tcg_gen_add_i64(cpu_R[rd], cpu_R[ra], cpu_R[rb]);
>>
>> Through this function you generate integer operations on the
>> registers, although ISA
>> suggests that there should be either single- or double-precision
>> floating point operations.
>>
>
> Sorry, I didn't find a TCG-IR that make single- or double-precision
> floating point operations, may you give me some hits?
As chenwj said it is usually done in helper functions.
Software floating point structures, prototypes and implementations
live in the fpu/ directory.
Good example of how to use them is MIPS:
target-mips/translate.c, function gen_farith and
target-mips/op_helper.c, macro FLOAT_BINOP
--
Thanks.
-- Max
- Re: [Qemu-devel] [PATCH 01/15] Openrisc: add target stub, (continued)
[Qemu-devel] [PATCH 03/15] Openrisc: add instructions translation, Jia Liu, 2012/05/17
Re: [Qemu-devel] [PATCH 03/15] Openrisc: add instructions translation, Blue Swirl, 2012/05/19
Re: [Qemu-devel] [PATCH 03/15] Openrisc: add instructions translation, Jia Liu, 2012/05/23
Re: [Qemu-devel] [PATCH 03/15] Openrisc: add instructions translation, Blue Swirl, 2012/05/23
Re: [Qemu-devel] [PATCH 03/15] Openrisc: add instructions translation, Jia Liu, 2012/05/25
Re: [Qemu-devel] [PATCH 03/15] Openrisc: add instructions translation, Jia Liu, 2012/05/25
[Qemu-devel] [PATCH 04/15] Openrisc: add interrupt support, Jia Liu, 2012/05/17