Re: [Qemu-devel] [PATCH v5 3/6] target: [tcg] Add generic translation framework

[Richard Henderson]

On 12/28/2016 08:28 AM, Lluís Vilanova wrote:
> +typedef enum DisasJumpType {
> +    DJ_NEXT,
> +    DJ_TOO_MANY,
> +    DJ_TARGET,
> +} DisasJumpType;

I wonder if enums like DJ_TARGET_{0..N} wouldn't be better, rather than doing 
addition in the target-specific names.

> +typedef struct DisasContextBase {
> +    TranslationBlock *tb;
> +    bool singlestep_enabled;
> +    target_ulong pc_first;
> +    target_ulong pc_next;
> +    DisasJumpType jmp_type;
> +    unsigned int num_insns;
> +} DisasContextBase;

Sort the bool to the end to minimize padding.

> +/* Get first breakpoint matching a PC */
> +static inline CPUBreakpoint *cpu_breakpoint_get(CPUState *cpu, vaddr pc,
> +                                                CPUBreakpoint *bp)
> +{
> +    if (likely(bp == NULL)) {
> +        if (unlikely(!QTAILQ_EMPTY(&cpu->breakpoints))) {
> +            QTAILQ_FOREACH(bp, &cpu->breakpoints, entry) {
> +                if (bp->pc == pc) {
> +                    return bp;
> +                }
> +            }
> +        }
> +    } else {
> +        QTAILQ_FOREACH_CONTINUE(bp, entry) {
> +            if (bp->pc == pc) {
> +                return bp;
> +            }
> +        }
> +    }
> +    return NULL;
> +}

Any reason not to put the QTAILQ_FOREACH directly into gen_intermediate_code, 
rather than indirect it like this?  I don't see this abstraction as an improvement.


r~