https://bugs.kde.org/show_bug.cgi?id=429352
--- Comment #10 from Julian Seward <jsew...@acm.org> --- ================== https://bugs.kde.org/attachment.cgi?id=133459 Test cases for new IOps OK to land ================== https://bugs.kde.org/attachment.cgi?id=135083 Fix pcast for existing code OK to land ================== https://bugs.kde.org/attachment.cgi?id=135176 Add ACC support into routine get_otrack_shadow_offset_wrk() OK to land ================== https://bugs.kde.org/attachment.cgi?id=135082 Functional support for ISA 3.1 New Iops OK to land -- I don't see any significant problems with it -- but first please fix the following comments. +UInt generate_DFP_FPRF_value_helper( UInt gfield, If this is intended to be called from VEX-generated code, add a comment to that effect, and add an extern qualifier. If it isn't, add a static qualifier. (== follow the existing conventions) +PPCInstr* PPCInstr_AvTernaryInt128 ( PPCAvOp op, HReg dst, + HReg src1, HReg src2, HReg src3 ) { + PPCInstr* i = LibVEX_Alloc_inline(sizeof(PPCInstr)); + i->tag = Pin_AvTrinaryInt128; + i->Pin.AvTrinaryInt128.op = op; There's still an inconsistency in the naming: Ternary vs Trinary. I'd prefer you use Ternary consistently, everywhere. @@ -2321,6 +2428,21 @@ void ppPPCInstr ( const PPCInstr* i, Bool mode64 ) ppHRegPPC(i->Pin.Dfp128Cmp.dst); vex_printf(",8,28,31"); return; + + case Pin_XFormUnary994: + if (Px_DFPTOIQS) { This isn't right. It needs to be if (i->Pin.XFormUnary994.op == Px_DFPTOIQS) or something like that. + case Pin_XFormUnary994: { + int inst_sel; Regarding "int", please use the project-defined types, always: Int, Char, UChar etc, not "int", "char" Also, here, "inst_sel" is local to each of the two case-clauses, so move it into each. + case Iop_D128toI128S: { + HReg srcHi = newVRegF(env); + HReg srcLo = newVRegF(env); .. + iselDfp128Expr( &srcHi, &srcLo, env, e->Iex.Binop.arg2, IEndianess ); Isn't it the case that iselDfp128Expr always writes its first two arguments (meaning, it selects the output virtual registers itself) ? If so, then it's pointless to initialise srcHi/srcLo by calling newVRegF, because those vregs will be ignored. Better to initialise them with HReg_INVALID (or is it INVALID_HREG). Similar comment in some other places further down the file. + // store the two 64-bit pars pairs + HReg rHi, rLo; + HReg dst = newVRegV(env); + + iselInt128Expr(&rHi,&rLo, env, e->Iex.Unop.arg, IEndianess); For example here: now there's no initialisation at all. Initialise rHi/rLo to HREG_INVALID here. @@ -314,8 +316,10 @@ void ppIROp ( IROp op ) case Iop_F128LOtoF64: vex_printf("F128LOtoF64"); return; case Iop_I32StoF128: vex_printf("I32StoF128"); return; case Iop_I64StoF128: vex_printf("I64StoF128"); return; + case Iop_I128StoF128: vex_printf("128StoF128"); return; In the output text, the initial "I" is missing @@ -4753,15 +4769,19 @@ IRAtom* expr2vbits_Binop ( MCEnv* mce, uifu = mkUifU1; difd = mkDifD1; and_or_ty = Ity_I1; improve = mkImproveOR1; goto do_And_Or; - do_And_Or: + do_And_Or: Please restore the original indentation for this label. ================== -- You are receiving this mail because: You are watching all bug changes.
