Instruction registers display is buggy/unreliable

[Fish-Git]

---

Note: this issue is closely related to issue #33: "Trace-to-file feature".

---

When instruction tracing/stepping is enabled (t+ and s+ commands), Control, Floating Point and Access registers are not being displayed for some instructions which actually modify/use them. (See e.g. display_inst_regs function in hscmisc.c)

---

Edit: The problem also occurs when an instruction is displayed as a result of an interruption too, such as when e.g. an Operation Exception or Translation Specification Exception occurs. That is to say, the problem is not limited to just the t+ and s+ tracing/stepping commands. It is a general overall problem/issue with (design flaw in) the display_inst_regs function itself.

Having the Control Registers also displayed whenever a program interrupt occurs for example, can be quite valuable information to have, as it can tell you in which address space the problem occurred for example.

---

The problem stems from the fact that the display_inst_regs function is deciding whether or not to also display the "other" registers (CR, AR, FPR) based on tests for specific instruction opcodes, which overly complicates the logic and makes it it less reliable and harder to maintain (e.g. it's easy to overlook updating this function when new instructions are added to the architecture for example).

We need a more reliable, less error prone way to determine whether CR, AR, FPR should be traced or not for a given instruction. I'm thinking maybe an update to our existing OPCODE table might do the trick.

I propose adding (*) a new regsmask parameter to our existing GENx370x390x900 and related macros, that defines which registers the given instruction uses or modifies. Then the display_inst_regs function could then do a simple(?) table lookup to retrieve the mask, thereby reliably determining which registers should be displayed/traced.

---

(*) And adjusting all "opcode table" related code that might be impacted by such a change too of course. That is to say, I haven't researched yet the ramifications (impact) that adding a new field to our opcode table might cause.

[Fish-Git]

FYI to those interested: here's our current display_inst_regs function code:

hyperion/hscmisc.c

Lines 306 to 380 in 97ef4d5

	/*-------------------------------------------------------------------*/
	/* Display registers for the instruction display */
	/*-------------------------------------------------------------------*/
	int display_inst_regs (REGS *regs, BYTE *inst, BYTE opcode, char *buf, int buflen )
	{
	int len=0;
	/* Display the general purpose registers */
	if (!(opcode == 0xB3 || (opcode >= 0x20 && opcode <= 0x3F))
	|| (opcode == 0xB3 && (
	(inst[1] >= 0x80 && inst[1] <= 0xCF)
	|| (inst[1] >= 0xE1 && inst[1] <= 0xFE)
	)))
	{
	len += display_gregs (regs, buf + len, buflen - len - 1, "HHC02269I " );
	}
	/* Display control registers if appropriate */
	if (!REAL_MODE(&regs->psw) || opcode == 0xB2 || opcode == 0xB6 || opcode == 0xB7)
	{
	len += display_cregs (regs, buf + len, buflen - len - 1, "HHC02271I ");
	}
	/* Display access registers if appropriate */
	if (!REAL_MODE(&regs->psw) && ACCESS_REGISTER_MODE(&regs->psw))
	{
	len += display_aregs (regs, buf + len, buflen - len - 1, "HHC02272I ");
	}
	/* Display floating point control register if AFP enabled */
	if ((regs->CR(0) & CR0_AFP) && (
	(opcode == 0x01 && inst[1] == 0x0A) /* PFPO Perform Floating Point Operation */
	|| (opcode == 0xB2 && inst[1] == 0x99) /* SRNM Set BFP Rounding mode 2-bit */
	|| (opcode == 0xB2 && inst[1] == 0x9C) /* STFPC Store FPC */
	|| (opcode == 0xB2 && inst[1] == 0x9D) /* LFPC Load FPC */
	|| (opcode == 0xB2 && inst[1] == 0xB8) /* SRNMB Set BFP Rounding mode 3-bit */
	|| (opcode == 0xB2 && inst[1] == 0xB9) /* SRNMT Set DFP Rounding mode */
	|| (opcode == 0xB2 && inst[1] == 0xBD) /* LFAS Load FPC and Signal */
	|| (opcode == 0xB3 && (inst[1] <= 0x1F)) /* RRE BFP arithmetic */
	|| (opcode == 0xB3 && (inst[1] >= 0x40 && inst[1] <= 0x5F)) /* RRE BFP arithmetic */
	|| (opcode == 0xB3 && (inst[1] >= 0x84 && inst[1] <= 0x8C)) /* SFPC, SFASR, EFPC */
	|| (opcode == 0xB3 && (inst[1] >= 0x90 && inst[1] <= 0xAF)) /* RRE BFP arithmetic */
	|| (opcode == 0xB3 && (inst[1] >= 0xD0))/*inst[1] <= 0xFF)) */ /* RRE DFP arithmetic */
	|| (opcode == 0xB9 && (inst[1] >= 0x41 && inst[1] <= 0x43)) /* DFP Conversions */
	|| (opcode == 0xB9 && (inst[1] >= 0x49 && inst[1] <= 0x5B)) /* DFP Conversions */
	|| (opcode == 0xED && (inst[1] <= 0x1F)) /* RXE BFP arithmetic */
	|| (opcode == 0xED && (inst[1] >= 0x40 && inst[1] <= 0x59)) /* RXE DFP shifts, tests*/
	|| (opcode == 0xED && (inst[1] >= 0xA8 && inst[1] <= 0xAF))) /* RXE DFP conversions */
	)
	{
	len += snprintf(buf + len, buflen - len, MSG(HHC02276,"I", regs->fpc));
	}
	/* Display floating-point registers if appropriate */
	if ( (opcode == 0xB3 && !((inst[1] == 0x84) || (inst[1] == 0x85) || (inst[1] == 0x8C))) /* exclude FPC-only instrs */
	|| (opcode == 0xED)
	|| (opcode >= 0x20 && opcode <= 0x3F) /* HFP Arithmetic and load/store */
	|| (opcode >= 0x60 && opcode <= 0x70) /* HFP Arithmetic and load/store */
	|| (opcode >= 0x78 && opcode <= 0x7F) /* HFP Arithmetic and load/store */
	|| (opcode == 0xB2 && inst[1] == 0x2D) /* DXR Divide HFP extended */
	|| (opcode == 0xB2 && inst[1] == 0x44) /* SQDR Square Root HFP long */
	|| (opcode == 0xB2 && inst[1] == 0x45) /* SQER Square Root HFP short */
	|| (opcode == 0xB9 && (inst[1] >= 0x41 && inst[1] <= 0x43)) /* DFP Conversions*/
	|| (opcode == 0xB9 && (inst[1] >= 0x49 && inst[1] <= 0x5B)) /* DFP Conversions*/
	|| (opcode == 0x01 && inst[1] == 0x0A) /* PFPO Perform Floating Point Operation */
	)
	{
	len += display_fregs (regs, buf + len, buflen - len - 1, "HHC02270I ");
	}
	if (len && sysblk.showregsfirst)
	len += snprintf( buf + len, buflen - len, "\n" );
	return len;
	}

Is that crazy or what?!

[Fish-Git]

And the following trace display (created with TRACEOPT REGSFIRST active) perfectly illustrates the problem:

HHC02269I IP05: R0=0000000001000002 R1=0000001200002B80 R2=0000000000000000 R3=00000010C0000000
HHC02269I IP05: R4=0000000002F48500 R5=0000000002F48000 R6=0000000000000064 R7=0000000001DB9E00
HHC02269I IP05: R8=00000000030E0000 R9=00000000030E0000 RA=000000000004054A RB=00000000030E003D
HHC02269I IP05: RC=0000000001913A48 RD=00000000030E0B90 RE=0000000001910F28 RF=000000000017CF91
HHC02271I IP05: C0=00800002DF98E620 C1=00000001FFE4C007 C2=000000007D403BC0 C3=0000000180400013
HHC02271I IP05: C4=0000000100000013 C5=000000007EF354C0 C6=0000000002000000 C7=00000001FFE4C007
HHC02271I IP05: C8=0000000000020000 C9=0000000000000000 CA=0000000000000000 CB=0000000000000000
HHC02271I IP05: CC=0000000104609193 CD=00000001FFE4C007 CE=00000000DF8FEF34 CF=0000000002B37010
HHC02272I IP05: AR00=00000000 AR01=00000000 AR02=00000000 AR03=00000000
HHC02272I IP05: AR04=01000002 AR05=01000002 AR06=00000000 AR07=01000002
HHC02272I IP05: AR08=00000000 AR09=00000000 AR10=00000000 AR11=00000000
HHC02272I IP05: AR12=00000000 AR13=00000000 AR14=00000000 AR15=07000000
HHC02324I IP05: PSW=0404600180000000 00000000019125DC INST=B24E0010  SAR 1,0   set_access_register
HHC02326I IP05: V:0000001200002B80: Translation exception 003B (Region-third-translation exception)
HHC02326I IP05: V:0000000001000002: Translation exception 0004 (Protection exception)

As you can clearly see, the SAR instruction (which is an RRE (register to register) format instruction) is not only mistakenly being interpreted by our instruction tracing logic as a storage-to-storage instruction (thus trying to display storage when it shouldn't be), but is also displaying Control Registers when the instruction clearly has absolutely nothing to do with Control Registers.

If our opcode table contained more information about each instruction (like which registers it used, which operand(s) referenced storage, etc), our instruction tracing logic could use it to more accurately know what information it should be tracing.

As it is today, we're wasting valuable host CPU cycles and log file disk space formatting and displaying unnecessary information, which slows down instruction tracing significantly.