On 5/30/19 2:07 PM, Michael Rolnik wrote:
> + /* decode first instruction */
> + ctx.inst[0].cpc = pc_start;
> + decode_opc(&ctx, &ctx.inst[0]);
> + do {
> + /* set curr/next PCs */
> + cpc = ctx.inst[0].cpc;
> + npc = ctx.inst[0].npc;
> +
> + /* decode next instruction */
> + ctx.inst[1].cpc = ctx.inst[0].npc;
> + decode_opc(&ctx, &ctx.inst[1]);
> +
> + /* translate current instruction */
> + tcg_gen_insn_start(cpc);
> + num_insns++;
I don't believe that this simultaneous decode of two instructions is correct.
Consider if ctx.inst[0] is a branch instruction that is placed as the very last
word of memory. Ordinarily, the branch would be executed and the
TranslationBlock ended.
However, the advance read of ctx.inst[1] will cause a read from unmapped
address space (causing an exception), or read from a device (causing "Bad ram
pointer" and an abort from qemu_ram_addr_from_host_nofail).
I believe that the feature that you're attempting to support with this, skip
the next instruction, should be handled via an internal flag bit. This would
end up looking a lot like the HPPA nullify bit, or somewhat like the ARM thumb
condexec_mask. I can go into specifics if needed.
Such a change would also allow you to structure this code to use
"exec/translator.h", which in the future will likely be mandatory.
r~