On 09/08/2016 03:31 PM, Michael Rolnik wrote:
+#define CPU_IMM(env) ((env)->r[62])
+#define CPU_PCL(env) ((env)->r[63])
You don't need to represent these as actual registers. These are better
considered placeholder regnums to be filled in with actual values during
translation.
+struct CPUARCState {
+ uint32_t r[64];
+
+ struct {
+ uint32_t Lf;
+ uint32_t Zf; /* zero */
+ uint32_t Nf; /* negative */
+ uint32_t Cf; /* carry */
+ uint32_t Vf; /* overflow */
+ uint32_t Uf;
+
+ uint32_t DEf;
+ uint32_t AEf;
+ uint32_t A2f; /* interrupt 1 is active */
+ uint32_t A1f; /* interrupt 2 is active */
+ uint32_t E2f; /* interrupt 1 mask */
+ uint32_t E1f; /* interrupt 2 mask */
+ uint32_t Hf; /* halt */
+ } stat, stat_l1, stat_l2, stat_er;
There is no reason to represent each bit as a whole word, and even less to have
four copies.
Only the current NZCV bits are relevant for expansion to a word, and even then
you should consider not canonicalizing N, Z and V to a pure boolean value.
+
+ struct {
+ uint32_t S2;
+ uint32_t S1;
+ uint32_t CS;
+ } macmod;
Does CS really need to be represented at all? It appears to me to be a field
that you write to that clears S1 and S2.
+ switch (n) {
+ case 0x00 ... 0x3f: {
+ val = env->r[n];
+ break;
+ }
Please use the proper format for all switch statements.
switch (n) {
case 0x00 ... 0x3f:
val = env->r[n];
break;
+
+TCGv_env cpu_env;
+
+TCGv cpu_gp; /* Global Pointer */
+TCGv cpu_fp; /* Frame Pointer */
+TCGv cpu_sp; /* Stack Pointer */
+TCGv cpu_ilink1; /* Level 1 interrupt link register */
+TCGv cpu_ilink2; /* Level 2 interrupt link register */
+TCGv cpu_blink; /* Branch link register */
+TCGv cpu_mlo; /* Multiply low 32 bits, read only */
+TCGv cpu_mmi; /* Multiply middle 32 bits, read only */
+TCGv cpu_mhi; /* Multiply high 32 bits, read only */
Why are these separate variables? They overlap cpu_r[N]. If you want them as
names for clarity in translation, #define seems good enough.
+int arc_gen_INVALID(DisasCtxt *ctx)
+{
+ printf("invalid inst @:%08x\n", ctx->cpc);
No printf. It's not useful, even temporarily.
+ ctx.zero = tcg_const_local_i32(0);
+ ctx.one = tcg_const_local_i32(1);
+ ctx.msb32 = tcg_const_local_i32(0x80000000);
+ ctx.msb16 = tcg_const_local_i32(0x00008000);
+ ctx.smax16 = tcg_const_local_i32(0x7fffffff);
+ ctx.smax32 = tcg_const_local_i32(0x00007fff);
+ ctx.smax5 = tcg_const_local_i32(0x0000001f);
+ ctx.smin5 = tcg_const_local_i32(0xffffffe1);
Why are you creating all of these? Creating local temps containing immediates
is a horrible waste.
+ if (ctx.npc == env->lpe) {
You can't look at the contents of ENV during translation.
You'll need to implement this feature similar to how it's done for xtensa. See
helper_wsr_lbeg, helper_wsr_lend, and gen_check_loop_end.
r~
