Re: [Qemu-devel] [PATCH] target-tilegx: Execute _start and reach to __libc_start_main successfully

[Chris Metcalf <cmetcalf@ezchip.com>]

On 2/24/2015 11:31 AM, Chen Gang S wrote:
>   - When dst is zero, in most cases, we can just skip the insn, but in
>     some cases, it may cause exception in user mode (e.g st zero r0).

Be careful of skipping instructions in general, since for example a "ld zero, r1" may be targeting an MMIO address that we are reading to trigger some device operation, but don't need the result from.

> - For add/addi operation, it will change to mov/movi operation.
>
>   - For mov operation, it will change to movi operation.
>
>   - For shl3add, if srcb is zero register, it will change to shli
>     operation.

I assume you are referring to missed performance optimization opportunities if you don't treat "zero" specially?  You certainly could treat "add r1, r2, zero" as just an "add" instruction anyway, just with a zero addend.  You don't have to convert it to a move instruction.  Likewise with the others.

> OK, thanks.  originally I wanted to reuse them, but after think of, I
> prefer the 64-bit immediate number instead of.
>
>   - The decoding function may be very long, but it is still a simple
>     function, I guess, it is still simple enough for readers.
>
>   - 64-bit immediate number has better performance under 64-bit machine
>     (e.g x86-64).

What I mean is you should just directly use all those accessor functions.  Then your code would look like

switch (get_Opcode_X1(bundle)) {   // this is a 59-bit shift and mask by 0x7
case SHL16INSLI_OPCODE_X1:   // this is the constant 7
   [...]
}

Typically dealing with smaller integers is higher-performance on any platform, I suspect, even on x86 when you can have large inline constants in the code.  The compiler might be smart enough to do this for you, to be fair.  In any case any performance difference of this switch is almost certainly lost in the noise.

More to the point, named constants are almost always better for code maintainability than raw integers in code.

Also, my point is that you should just include a verbatim copy of the kernel header into qemu, and then use those names.  This is helpful to people who are already familiar with the <arch/opcode.h> naming, and reduces the amount of code needed to review qemu in any case.

-- 
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com