Valgrind on s390x currently lacks support for the miscellaneous instruction extensions facility 2. It consists of the following new instructions: AGH -- add halfword to 64 bit value BIC -- branch indirect on condition MGRK -- multiply 64x64reg -> 128 MG -- multiply 64x64mem -> 128 MGH -- multiply halfword 64x16mem -> 64 MSRKC -- multiply single 32x32 -> 32 MSGRKC -- multiply single 64x64 -> 64 MSC -- multiply single 32x32mem -> 32 MSGC -- multiply single 64x64mem -> 64 SGH -- subtract halfword from 64 bit value
Created attachment 120110 [details] Patches for z14 miscellaneous instruction support These patches should provide the functionality requested in this Bug. In summary, they address the following: Patch #1: Adds the new machine models z14 and z14 ZR1. Patch #2: Cleans up s390-check-opcodes.pl, to fix false positives when checking s390-opcodes.csv. Patch #3: Implements the requested instructions. Patch by Ilya Leoshkevich. Patch #4: Adds tests for the new instructions. Patch by Ilya Leoshkevich.
(In reply to Andreas Arnez from comment #1) > Patch #1: Adds the new machine models z14 and z14 ZR1. > Patch #2: Cleans up s390-check-opcodes.pl, to fix false positives when > checking s390-opcodes.csv. > Patch #4: Adds tests for the new instructions. Patch by Ilya Leoshkevich. These 3 look fine; OK to land. > Patch #3: Implements the requested instructions. Patch by Ilya Leoshkevich. One minor and one maybe not so minor thing. First the small thing: --- a/VEX/priv/guest_s390_toIR.c +++ b/VEX/priv/guest_s390_toIR.c +static const HChar * +s390_irgen_BIC(UChar r1, IRTemp op2addr) +{ + IRTemp cond = newTemp(Ity_I32); + + if (r1 == 0) { + } else if (r1 == 15) { I assume the empty { } for the case r1 == 0 is intended, yes? If so please put " /* nothing */" inside the { } so as to make this clear. The maybe not so minor thing is: In host_s390_isel.c, is ISelEnv::hwcaps checked before creating a MG or MGRK instruction? If it isn't, and the host doesn't have that capability, but by mistake a MG/MGRK instruction is created, then the system will fail with SIGILL on the host (I assume). If the check is in place then at least we'll get an assertion from somewhere inside host_s390_isel.c, saying "I can't reduce this tree" (select insns for it).
Created attachment 120343 [details] Fixup that adds /* nothing */ and hwcaps check
(In reply to Ilya Leoshkevich from comment #3) > Created attachment 120343 [details] > Fixup that adds /* nothing */ and hwcaps check Julian, I think this patch addresses your concern with accidentally generating an MG or MGRK instruction. It also adds the comment, as requested. Do you see anything else we may have missed?
(In reply to Andreas Arnez from comment #4) > (In reply to Ilya Leoshkevich from comment #3) Andreas, Ilya, thanks for the fixes. regarding this: --- a/VEX/priv/host_s390_isel.c +++ b/VEX/priv/host_s390_isel.c @@ -1018,6 +1018,7 @@ s390_isel_int128_expr_wrk(HReg *dst_hi, HReg *dst_lo, ISelEnv *env, goto do_multiply64; case Iop_MullS64: + vassert(env->hwcaps & VEX_HWCAPS_S390X_MI2); is_signed_multiply = True; goto do_multiply64; Hmm, sorry. I should have been more precise. Two changes instead of the above assertion: (1) At the end of s390_isel_int128_expr_wrk(), add a label "irreducible:" and two lines following it. Copy them from the same at the end of s390_isel_int_expr_wrk. (2) Where your diff above has an assert, remove it and instead add if (!(env->hwcaps & VEX_HWCAPS_S390X_MI2))) goto irreducible; That's generally how the instruction selectors deal with trees that they can't reduce.
Created attachment 120373 [details] updated fixup Hi Julian, thanks for the explanation. I've added the label.
Since Ilya has changed the assertion as requested, are there any more comments? If nobody objects, I'd like to push this early next week.
Since there were no further comments, I pushed the following commits: 65d8e9ed9 s390x: Add models "z14" and "z14 ZR1" e63f93a97 s390x: Clean up s390-check-opcodes.pl 50b20aa24 Bug 404406 - s390x: implement z14 miscellaneous instructions 91d53d116 Bug 404406 - s390x: test z14 miscellaneous instructions