1. The code was generated. Every instruction has its own extractors. there
are several reasons
1. I don't have to think and gather the instructions into the group.
2. I don't have to remember what group an instruction is in
3. there is no performance penalty.
2. done.
On Sun, Jun 5, 2016 at 1:17 AM, Richard Henderson <r...@twiddle.net> wrote:
> On 06/02/2016 01:06 PM, Michael Rolnik wrote:
>
>> Signed-off-by: Michael Rolnik <mrol...@gmail.com>
>> ---
>> target-avr/translate-inst.h | 838
>> ++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 838 insertions(+)
>> create mode 100644 target-avr/translate-inst.h
>>
>> diff --git a/target-avr/translate-inst.h b/target-avr/translate-inst.h
>> new file mode 100644
>> index 0000000..6bd180d
>> --- /dev/null
>> +++ b/target-avr/translate-inst.h
>> @@ -0,0 +1,838 @@
>> +
>> +#ifndef AVR_TRANSLATE_INST_H_
>> +#define AVR_TRANSLATE_INST_H_
>> +
>> +typedef struct DisasContext DisasContext;
>> +
>> +int avr_translate_NOP(CPUAVRState *env, DisasContext* ctx, uint32_t
>> opcode);
>> +
>> +int avr_translate_MOVW(CPUAVRState *env, DisasContext* ctx, uint32_t
>> opcode);
>> +static inline uint32_t MOVW_Rr(uint32_t opcode)
>> +{
>> + return extract32(opcode, 0, 4);
>> +}
>> +static inline uint32_t MOVW_Rd(uint32_t opcode)
>> +{
>> + return extract32(opcode, 4, 4);
>> +}
>> +
>> +int avr_translate_MULS(CPUAVRState *env, DisasContext* ctx, uint32_t
>> opcode);
>> +static inline uint32_t MULS_Rr(uint32_t opcode)
>> +{
>> + return extract32(opcode, 0, 4);
>> +}
>> +static inline uint32_t MULS_Rd(uint32_t opcode)
>> +{
>> + return extract32(opcode, 4, 4);
>> +}
>>
>
> Any particular reason you're replicating all this logic for every
> instruction, as opposed to finding some commonalty between them? There
> aren't *that* many different operand field types.
>
> Glancing through the binutils source, I count 13. Although I might have
> missed a handful, it's still an order of magnitude less than your 159
> functions.
>
> +static inline uint32_t CPC_lRr(uint32_t opcode)
>> +{
>> + return extract32(opcode, 0, 4);
>> +}
>>
> ...
>
>> +static inline uint32_t CPC_hRr(uint32_t opcode)
>> +{
>> + return extract32(opcode, 9, 1);
>> +}
>>
>
> Any reason you're not simplifying the logic elsewhere by extracting the
> logical value, as opposed to the parts of the split value. I.e.
>
> (extract32(opcode, 9, 1) << 4) | extract32(opcode, 0, 4)
>
>
> r~
>
--
Best Regards,
Michael Rolnik
