Re: [Qemu-ppc] [PATCH 5/8] target-ppc: Load Quadword

[Tom Musta]

On 1/27/2014 12:55 PM, Alexander Graf wrote:
> You're mixing two semantically separate things here. legal_in_user_mode 
> doesn't really indicate that le_mode isn't usable. I'm sure if you just make 
> this two if()'s with two separate bools that get assigned the same value gcc 
> will be smart enough to optimize it just as well as this combined branch.
> 

Hmmm ... I'm not sure that I see the problem.  Perhaps the comment should be 
clearer.
And I guess there is really no need to compute the legal_in_user_mode flag 
since it
is only used once.

Prior to ISA 2.07, lq was not legal in user mode; attempting to execute lq when 
MSR[PR]=1
resulted in a privileged instruction exception.  Also, when MSR[PR]=0 and 
MSR[LE]=1, an
alignment exception was generated irrespective of the computed address.

Starting with ISA 2.07, both of these restrictions are lifted.  So the proposed 
code is
as follows:

static void gen_lq(DisasContext *ctx)
{
    /* lq is a legal user mode instruction starting in ISA 2.07 */
    int legal_in_user_mode = (ctx->insns_flags2 & PPC2_LSQ_ISA207) != 0;

    if (!legal_in_user_mode) {
#if defined(CONFIG_USER_ONLY)
        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
        return;
#else
        if (unlikely(ctx->mem_idx == 0)) {
            gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
            return;
        }

        if (unlikely(ctx->le_mode)) {
            /* Little-endian mode is not handled */
            gen_exception_err(ctx, POWERPC_EXCP_ALIGN, POWERPC_EXCP_ALIGN_LE);
            return;
        }
#endif
    }

    int ra, rd;
    TCGv EA;
    ... // rest of implementation


P.S.  I think there should be an alignment check after the EA is computed.