Re: [PATCH v32 04/13] target/avr: Add instruction translation - Registers definition

[Aleksandar Markovic]

On Friday, October 18, 2019, Michael Rolnik <address@hidden> wrote:

On Fri, Oct 18, 2019 at 11:52 AM Aleksandar Markovic <address@hidden> wrote:

On Thursday, October 17, 2019, Michael Rolnik <address@hidden> wrote:

On Thu, Oct 17, 2019 at 11:17 PM Aleksandar Markovic
<address@hidden> wrote:
>>
>>
>> >> +static TCGv cpu_Cf;
>> >> +static TCGv cpu_Zf;
>> >> +static TCGv cpu_Nf;
>> >> +static TCGv cpu_Vf;
>> >> +static TCGv cpu_Sf;
>> >> +static TCGv cpu_Hf;
>> >> +static TCGv cpu_Tf;
>> >> +static TCGv cpu_If;
>> >> +
>> >
>> >
>> > Hello, Michael,
>> >
>> > Is there any particular reason or motivation beyond modelling status register flags as TCGv variables?
>>
>>
>>
>> I think it's easier this way as I don't need to convert flag values to
>> bits or bits to flag values.
>
>
> Ok. But, how do you map 0/1 flag value to the value of a TCGv variable and vice versa? In other words, what value or values (out of 2^32 vales) of a TCGv variable mean the flag is 1? And the same question for 0.
>
> Is 0110000111000010100 one or zero?
>
> Besides, in such arrangement, how do you display the 8-bit status register in gdb, if at all?

each flag register is either 0 or 1,....

Michael,

If this is true, why is there a special handling of two flags in the following code:

static inline uint8_t cpu_get_sreg(CPUAVRState *env)
	{
	uint8_t sreg;
	sreg = (env->sregC & 0x01) << 0
	| (env->sregZ == 0 ? 1 : 0) << 1
	| (env->sregN) << 2
	| (env->sregV) << 3
	| (env->sregS) << 4
	| (env->sregH) << 5
	| (env->sregT) << 6
	| (env->sregI) << 7;
	return sreg;
	}
	static inline void cpu_set_sreg(CPUAVRState *env, uint8_t sreg)
	{
	env->sregC = (sreg >> 0) & 0x01;
	env->sregZ = (sreg >> 1) & 0x01 ? 0 : 1;
	env->sregN = (sreg >> 2) & 0x01;
	env->sregV = (sreg >> 3) & 0x01;
	env->sregS = (sreg >> 4) & 0x01;
	env->sregH = (sreg >> 5) & 0x01;
	env->sregT = (sreg >> 6) & 0x01;
	env->sregI = (sreg >> 7) & 0x01;
	}

 ?

Aleksandar,

If I understand your question correctly cpu_get_sreg assembles SREG value to be presented by GDB, and cpu_set_sreg sets flags values when GDB modifies SREG.

Michael

Why is handling of sregC and sregZ flags different than handling of other flags? This contradicts your previos statement that 1 (in TCGv) means 1 (flag), and 0 (in TCGv) means 0 (flag)?

Whatever is the explanation, ot should be included, in my opinion, in code comments.

Please, Michael, let's first clarify the issue from the question above.

Thanks, Aleksandar

Thanks,

A.

 they are calculated here
1. https://github.com/michaelrolnik/qemu-avr/blob/avr-v32/target/avr/translate.c#L146-L148
2. https://github.com/michaelrolnik/qemu-avr/blob/avr-v32/target/avr/translate.c#L166
3. https://github.com/michaelrolnik/qemu-avr/blob/avr-v32/target/avr/translate.c#L185-L187
4. https://github.com/michaelrolnik/qemu-avr/blob/avr-v32/target/avr/translate.c#L205
5. https://github.com/michaelrolnik/qemu-avr/blob/avr-v32/target/avr/translate.c#L214-L215
6. https://github.com/michaelrolnik/qemu-avr/blob/avr-v32/target/avr/translate.c#L222-L223
The COU itself never uses SREG at all, only the flags.

As for the GDB it's get assembled/disassembled here
1. https://github.com/michaelrolnik/qemu-avr/blob/avr-v32/target/avr/cpu.h#L219-L243
2. https://github.com/michaelrolnik/qemu-avr/blob/avr-v32/target/avr/gdbstub.c#L35-L37
3. https://github.com/michaelrolnik/qemu-avr/blob/avr-v32/target/avr/gdbstub.c#L66-L68

>
> A.
>
>>
>> >
>> > A.
>> >
>> >
>> >
>> >>
>> >> +static TCGv cpu_rampD;
>> >> +static TCGv cpu_rampX;
>> >> +static TCGv cpu_rampY;
>> >> +static TCGv cpu_rampZ;
>> >> +
>> >> +static TCGv cpu_r[NO_CPU_REGISTERS];
>> >> +static TCGv cpu_eind;
>> >> +static TCGv cpu_sp;
>> >> +
>> >> +static TCGv cpu_skip;
>> >> +
>> >> +static const char reg_names[NO_CPU_REGISTERS][8] = {
>> >> +    "r0",  "r1",  "r2",  "r3",  "r4",  "r5",  "r6",  "r7",
>> >> +    "r8",  "r9",  "r10", "r11", "r12", "r13", "r14", "r15",
>> >> +    "r16", "r17", "r18", "r19", "r20", "r21", "r22", "r23",
>> >> +    "r24", "r25", "r26", "r27", "r28", "r29", "r30", "r31",
>> >> +};
>> >> +#define REG(x) (cpu_r[x])
>> >> +
>> >> +enum {
>> >> +    DISAS_EXIT   = DISAS_TARGET_0,  /* We want return to the cpu main loop.  */
>> >> +    DISAS_LOOKUP = DISAS_TARGET_1,  /* We have a variable condition exit.  */
>> >> +    DISAS_CHAIN  = DISAS_TARGET_2,  /* We have a single condition exit.  */
>> >> +};
>> >> +
>> >> +typedef struct DisasContext DisasContext;
>> >> +
>> >> +/* This is the state at translation time. */
>> >> +struct DisasContext {
>> >> +    TranslationBlock *tb;
>> >> +
>> >> +    CPUAVRState *env;
>> >> +    CPUState *cs;
>> >> +
>> >> +    target_long npc;
>> >> +    uint32_t opcode;
>> >> +
>> >> +    /* Routine used to access memory */
>> >> +    int memidx;
>> >> +    int bstate;
>> >> +    int singlestep;
>> >> +
>> >> +    TCGv skip_var0;
>> >> +    TCGv skip_var1;
>> >> +    TCGCond skip_cond;
>> >> +    bool free_skip_var0;
>> >> +};
>> >> +
>> >> +static int to_A(DisasContext *ctx, int indx) { return 16 + (indx % 16); }
>> >> +static int to_B(DisasContext *ctx, int indx) { return 16 + (indx % 8); }
>> >> +static int to_C(DisasContext *ctx, int indx) { return 24 + (indx % 4) * 2; }
>> >> +static int to_D(DisasContext *ctx, int indx) { return (indx % 16) * 2; }
>> >> +
>> >> +static uint16_t next_word(DisasContext *ctx)
>> >> +{
>> >> +    return cpu_lduw_code(ctx->env, ctx->npc++ * 2);
>> >> +}
>> >> +
>> >> +static int append_16(DisasContext *ctx, int x)
>> >> +{
>> >> +    return x << 16 | next_word(ctx);
>> >> +}
>> >> +
>> >> +
>> >> +static bool avr_have_feature(DisasContext *ctx, int feature)
>> >> +{
>> >> +    if (!avr_feature(ctx->env, feature)) {
>> >> +        gen_helper_unsupported(cpu_env);
>> >> +        ctx->bstate = DISAS_NORETURN;
>> >> +        return false;
>> >> +    }
>> >> +    return true;
>> >> +}
>> >> +
>> >> +static bool decode_insn(DisasContext *ctx, uint16_t insn);
>> >> +#include "decode_insn.inc.c"
>> >> +
>> >> --
>> >> 2.17.2 (Apple Git-113)
>> >>
>>
>>
>> --
>> Best Regards,
>> Michael Rolnik



--
Best Regards,
Michael Rolnik

--

Best Regards,
Michael Rolnik