Suggestion to improve readability

Juergen Buchmueller pullmoll at t-online.de
Sun Jun 3 22:08:35 PDT 2007


According to the docs section 3.4, page 30, the multiplexer and 16 way
dispatcher for IR is instrumented like this (with my comments in
parenthesis):

  if     IR[0] = 1     then 3-IR[8-9]	complement of SH field of IR
  elseif IR[1-2] = 0   then IR[3-4]	JMP, JSR, ISZ, DSZ
  elseif IR[1-2] = 1   then 4		LDA
  elseif IR[1-2] = 2   then 5		STA
  elseif IR[4-7] = 0   then 1		(this is CYCLE)
  elseif IR[4-7] = 1   then 0		(this is JSRII)
  elseif IR[4-7] = 6   then 16B		CONVERT (buggy comment!)
  elseif IR[4-7] = 16B then 6		(_this_ is CONVERT)
  else			    IR[4-7]	(remaining opcodes & TRAPS)

Note that IR[4-7] = x just means ((x & 017) << 010), so we'd have:
  0 << 010 = 0	   which is the CYCLE opcode (0060000)
 01 << 010 = 00400 which is the JSRII opcode (0064400)
 06 << 010 = 03000 which seems to be unused masks (TRAPS)
016 << 010 = 07000 which is the CONVERT opcode (067000)

I'd suggest to translate all this confusing stuff to C with the help of some
simplifying macros:

#define IR_MASK(from,to) \
	(ir & ((1<<(to+1-from))-1)<<(15-to))
#define	IR_EQU(from,to,val) \
	((ir & ((1<<(to+1-from))-1)<<(15-to)) == (val<<(15-to)))
#define	IR_GET(from,to) \
	((ir & ((1<<(to+1-from))-1))>>(15-to))

You'd of course have to add some more parenthesis around from, to, and
val, if they could be computed values. Here I assumed they are just
plain numbers.

Then you could, for a function like f2_fn_idisp_late(), have really nice
looking code, closely matching what the docs say:

  if (IR_MASK(0,0))
    bval = 3 - IR_GET(8,9);	/* complement of SH */
  else if (IR_EQU(1,2,0)) {
    bval = IR_GET(3,4);		/* jump group: JMP, JSR, ISZ, DSZ */
  else if (IR_EQU(1,2,1))
    bval = 4;			/* LDA */
  else if (IR_EQU(1,2,2))
    bval = 5;			/* STA */
  else if (IR_EQU(4,7,0))
    bval = 1;			/* CYCLE */
  else if (IR_EQU(4,7,1))
    bval = 0;			/* JSRII */
  else if (IR_EQU(4,7,6))
    bval = 016;			/* TRAPS */
  else if (IR_EQU(4,7,016))
    bval = 6;			/* CONVERT */
  else
    bval = IR_GET(4,7);		/* others and TRAPS */

It might even make sense to use this type of macros all over the place to
mask, compare, and extract bit fields from the general "reverse" notation
	REG[from-to]
that the Alto docs have. It seems to be just too easy to introduce bugs
when counting and flipping the bits manually. This is a re-appearing
problem ;-P

We could have some generalized macros ALTO_MASK(reg,from,to),
ALTO_EQU(reg,from,to,val), and ALTO_GET(reg,from,to).

And we could even define the standard "reg,from,to" fields as macros:
#define	IR_ARITHGRP	ir,0,0
#define	IR_SRCAC	ir,1,2
#define	IR_DSTAC	ir,3,4
#define	IR_ARITHOP	ir,5,7
#define	IR_SH		ir,8,9
#define	IR_CY		ir,10,11
#define	IR_NL		ir,12,12
#define	IR_SKIP		ir,13,15
and then just write ALTO_GET(IR_SH) to extract SH, or ALTO_EQU(IR_SH,1) to
compare it against 1 etc.

Hmm.. and a macro ALTO_PUT(reg,from,to,val) to write into a bit field could
prove just as useful.

Just my thoughts, since I was myself about to report a non-bug as a bug,
because I counted some bits wrong in this very function.

Juergen


More information about the Altogether-devel mailing list