Essentially, the way extended mnemonics are communicated to s390_disasm is a bit messy and, more importantly, error prone, as we have seen recently. One reason is that extended mnemonics were added late in the game and sort of force-fitted on top of the existing scheme. old: s390_disasm(ENC3(MNM, GPR, UINT), mnm, r1, i2); new: S390_DISASM(MNM(mnm), GPR(r1), UINT(i2)); old: s390_disasm(ENC2(XMNM, SDXB), S390_XMNM_BIC, r1, dh2, dl2, x2, b2); new: S390_DISASM(XMNM(mnm, bic_disasm), MASK(r1), SDXB(dh2, dl2, x2, b2)); The implementation in s390_disasm.h is straight forward. The order of S390_DISASM arguments is exactly as it is specified in POP. For opcodes that have extended mnemonics the 1st argument is an XMNM ctor taking the base mnemonic and a function pointer as arguments. The function is responsible for writing out the complete disassembled insn (not just the extended mnemonic). The changes in guest_s390_toIR.c and host_s390_defs.c are purely mechanical. But as I had to change essentially all s390_format_... and s390_emit_... I've taken the liberty to fix a few obviously incorrect things: In guest_s390_toIR.c: Fix core dump when disassembling an MVCRL insn. MNM is missing. s390_disasm(ENC2(UDXB, UDXB), mnm, d1, 0, b1, d2, 0, b2); Fix s390_format_VRRa_VVVMMM: m6 is not written because an UINT is missing s390_disasm(ENC6(MNM, VR, VR, VR, UINT, UINT), mnm, v1, v2, v3, m4, m5, m6); Affects: VFCE, VFCH, VFCHE, VFMIN, VFMAX In host_s390_defs.c: Fix s390_emit_VLR: s390_disasm(ENC3(MNM, VR, UDXB), "vlr", v1, v2); --> s390_disasm(ENC3(MNM, VR, VR), "vlr", v1, v2); Fix s390_emit_LOC: s390_disasm(ENC4(MNM, GPR, UINT, SDXB), "loc", r1, m3, dh2, dl2, 0, b2); - m3 is last operand - opcode has extended mnemonic Fix s390_emit_LOCG likewise Fix s390_emit_LOCGR: s390_disasm(ENC4(MNM, GPR, GPR, UINT), "locgr", r1, r2, m3); - opcode has extended mnemonic Fix s390_emit_LOCGHI: s390_disasm(ENC4(MNM, GPR, INT, UINT), "locghi", r1, (Int)(Short)i2, m3) - opcode has extended mnemonic Fix s390_emit_MVHHI: s390_disasm(ENC3(MNM, UDXB, INT), "mvhhi", d1, 0, b1, i2); - i2, an UShort, needs to be sign extended Tweak s390_emit_TM: s390_disasm(ENC3(MNM, UDXB, INT), "tm", d1, 0, b1, i2); - i2 is a mask; UINT is less confusing (makes no difference here) Fix s390_emit_TMY: s390_disasm(ENC3(MNM, SDXB, INT), "tmy", dh1, dl1, 0, b1, (Int)(Char)i2); - i2 is a mask; UINT is less confusing I did not bother with vector insns at this time. There are several issues there to be fixed. E.g. for s390_emit_VCEQ operand m5 is missing which isn't even available in that function... I'll do that once I have sorted out the equivalence classes wrt disassembly for the vector opcodes. Some overly long S390_DISASM lines are created. This is on purpose because it simplifies cross checking that the S390_DISASM invocations for a given opcode in guest_s390_toIR.c and host_s390_defs.c are identical. This patch needs C11 features (anonymous structs and unions).
Created attachment 177561 [details] s390_disasm rework patch
Fixed in c3049684e26ee4300c797a11c9b88708223771f3