| Message ID | 20220106112318.13864-1-victor.colombo@eldorado.org.br |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] target/ppc: Remove xscmpnedp instruction | expand |
On 1/6/22 12:23, Víctor Colombo wrote: > xscmpnedp was added in ISA v3.0 but removed in v3.0B. This patch > removes this instruction as it was not in the final version of v3.0. > > RFC to know if you think this is the correct approach. Usually we deprecate a feature for a minimum of two releases before removing it. It might be overkill for this case since the P9 processor implementation is based on v3.0B. I would simply remove the instruction since it never existed on any supported HW. I will wait for some more feedback. Reviewed-by: Cédric Le Goater <clg@kaod.org> Thanks, C. > Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br> > --- > target/ppc/fpu_helper.c | 1 - > target/ppc/helper.h | 1 - > target/ppc/translate/vsx-impl.c.inc | 1 - > target/ppc/translate/vsx-ops.c.inc | 1 - > 4 files changed, 4 deletions(-) > > diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c > index e5c29b53b8..f030858cf9 100644 > --- a/target/ppc/fpu_helper.c > +++ b/target/ppc/fpu_helper.c > @@ -2270,7 +2270,6 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, \ > VSX_SCALAR_CMP_DP(xscmpeqdp, eq, 1, 0) > VSX_SCALAR_CMP_DP(xscmpgedp, le, 1, 1) > VSX_SCALAR_CMP_DP(xscmpgtdp, lt, 1, 1) > -VSX_SCALAR_CMP_DP(xscmpnedp, eq, 0, 0) > > void helper_xscmpexpdp(CPUPPCState *env, uint32_t opcode, > ppc_vsr_t *xa, ppc_vsr_t *xb) > diff --git a/target/ppc/helper.h b/target/ppc/helper.h > index f9c72dcd50..8f02cabaf5 100644 > --- a/target/ppc/helper.h > +++ b/target/ppc/helper.h > @@ -400,7 +400,6 @@ DEF_HELPER_5(xsnmsubdp, void, env, vsr, vsr, vsr, vsr) > DEF_HELPER_4(xscmpeqdp, void, env, vsr, vsr, vsr) > DEF_HELPER_4(xscmpgtdp, void, env, vsr, vsr, vsr) > DEF_HELPER_4(xscmpgedp, void, env, vsr, vsr, vsr) > -DEF_HELPER_4(xscmpnedp, void, env, vsr, vsr, vsr) > DEF_HELPER_4(xscmpexpdp, void, env, i32, vsr, vsr) > DEF_HELPER_4(xscmpexpqp, void, env, i32, vsr, vsr) > DEF_HELPER_4(xscmpodp, void, env, i32, vsr, vsr) > diff --git a/target/ppc/translate/vsx-impl.c.inc b/target/ppc/translate/vsx-impl.c.inc > index c08185e857..fbef496257 100644 > --- a/target/ppc/translate/vsx-impl.c.inc > +++ b/target/ppc/translate/vsx-impl.c.inc > @@ -1092,7 +1092,6 @@ GEN_VSX_HELPER_X1(xstsqrtdp, 0x14, 0x06, 0, PPC2_VSX) > GEN_VSX_HELPER_X3(xscmpeqdp, 0x0C, 0x00, 0, PPC2_ISA300) > GEN_VSX_HELPER_X3(xscmpgtdp, 0x0C, 0x01, 0, PPC2_ISA300) > GEN_VSX_HELPER_X3(xscmpgedp, 0x0C, 0x02, 0, PPC2_ISA300) > -GEN_VSX_HELPER_X3(xscmpnedp, 0x0C, 0x03, 0, PPC2_ISA300) > GEN_VSX_HELPER_X2_AB(xscmpexpdp, 0x0C, 0x07, 0, PPC2_ISA300) > GEN_VSX_HELPER_R2_AB(xscmpexpqp, 0x04, 0x05, 0, PPC2_ISA300) > GEN_VSX_HELPER_X2_AB(xscmpodp, 0x0C, 0x05, 0, PPC2_VSX) > diff --git a/target/ppc/translate/vsx-ops.c.inc b/target/ppc/translate/vsx-ops.c.inc > index c974324c4c..67fa7b2e41 100644 > --- a/target/ppc/translate/vsx-ops.c.inc > +++ b/target/ppc/translate/vsx-ops.c.inc > @@ -197,7 +197,6 @@ GEN_XX3FORM_NAME(xsnmsubdp, "xsnmsubmdp", 0x04, 0x17, PPC2_VSX), > GEN_XX3FORM(xscmpeqdp, 0x0C, 0x00, PPC2_ISA300), > GEN_XX3FORM(xscmpgtdp, 0x0C, 0x01, PPC2_ISA300), > GEN_XX3FORM(xscmpgedp, 0x0C, 0x02, PPC2_ISA300), > -GEN_XX3FORM(xscmpnedp, 0x0C, 0x03, PPC2_ISA300), > GEN_XX3FORM(xscmpexpdp, 0x0C, 0x07, PPC2_ISA300), > GEN_VSX_XFORM_300(xscmpexpqp, 0x04, 0x05, 0x00600001), > GEN_XX2IFORM(xscmpodp, 0x0C, 0x05, PPC2_VSX), >
On Thu, 6 Jan 2022 13:21:46 +0100 Cédric Le Goater <clg@kaod.org> wrote: > On 1/6/22 12:23, Víctor Colombo wrote: > > xscmpnedp was added in ISA v3.0 but removed in v3.0B. This patch > > removes this instruction as it was not in the final version of v3.0. > > > > RFC to know if you think this is the correct approach. > > Usually we deprecate a feature for a minimum of two releases before > removing it. It might be overkill for this case since the P9 processor > implementation is based on v3.0B. > > I would simply remove the instruction since it never existed on any > supported HW. I will wait for some more feedback. > I don't think it makes sense to keep this instruction if it only existed in pre-GA HW. Acked-by: Greg Kurz <groug@kaod.org> > Reviewed-by: Cédric Le Goater <clg@kaod.org> > > Thanks, > > C. > > > > Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br> > > --- > > target/ppc/fpu_helper.c | 1 - > > target/ppc/helper.h | 1 - > > target/ppc/translate/vsx-impl.c.inc | 1 - > > target/ppc/translate/vsx-ops.c.inc | 1 - > > 4 files changed, 4 deletions(-) > > > > diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c > > index e5c29b53b8..f030858cf9 100644 > > --- a/target/ppc/fpu_helper.c > > +++ b/target/ppc/fpu_helper.c > > @@ -2270,7 +2270,6 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, \ > > VSX_SCALAR_CMP_DP(xscmpeqdp, eq, 1, 0) > > VSX_SCALAR_CMP_DP(xscmpgedp, le, 1, 1) > > VSX_SCALAR_CMP_DP(xscmpgtdp, lt, 1, 1) > > -VSX_SCALAR_CMP_DP(xscmpnedp, eq, 0, 0) > > > > void helper_xscmpexpdp(CPUPPCState *env, uint32_t opcode, > > ppc_vsr_t *xa, ppc_vsr_t *xb) > > diff --git a/target/ppc/helper.h b/target/ppc/helper.h > > index f9c72dcd50..8f02cabaf5 100644 > > --- a/target/ppc/helper.h > > +++ b/target/ppc/helper.h > > @@ -400,7 +400,6 @@ DEF_HELPER_5(xsnmsubdp, void, env, vsr, vsr, vsr, vsr) > > DEF_HELPER_4(xscmpeqdp, void, env, vsr, vsr, vsr) > > DEF_HELPER_4(xscmpgtdp, void, env, vsr, vsr, vsr) > > DEF_HELPER_4(xscmpgedp, void, env, vsr, vsr, vsr) > > -DEF_HELPER_4(xscmpnedp, void, env, vsr, vsr, vsr) > > DEF_HELPER_4(xscmpexpdp, void, env, i32, vsr, vsr) > > DEF_HELPER_4(xscmpexpqp, void, env, i32, vsr, vsr) > > DEF_HELPER_4(xscmpodp, void, env, i32, vsr, vsr) > > diff --git a/target/ppc/translate/vsx-impl.c.inc b/target/ppc/translate/vsx-impl.c.inc > > index c08185e857..fbef496257 100644 > > --- a/target/ppc/translate/vsx-impl.c.inc > > +++ b/target/ppc/translate/vsx-impl.c.inc > > @@ -1092,7 +1092,6 @@ GEN_VSX_HELPER_X1(xstsqrtdp, 0x14, 0x06, 0, PPC2_VSX) > > GEN_VSX_HELPER_X3(xscmpeqdp, 0x0C, 0x00, 0, PPC2_ISA300) > > GEN_VSX_HELPER_X3(xscmpgtdp, 0x0C, 0x01, 0, PPC2_ISA300) > > GEN_VSX_HELPER_X3(xscmpgedp, 0x0C, 0x02, 0, PPC2_ISA300) > > -GEN_VSX_HELPER_X3(xscmpnedp, 0x0C, 0x03, 0, PPC2_ISA300) > > GEN_VSX_HELPER_X2_AB(xscmpexpdp, 0x0C, 0x07, 0, PPC2_ISA300) > > GEN_VSX_HELPER_R2_AB(xscmpexpqp, 0x04, 0x05, 0, PPC2_ISA300) > > GEN_VSX_HELPER_X2_AB(xscmpodp, 0x0C, 0x05, 0, PPC2_VSX) > > diff --git a/target/ppc/translate/vsx-ops.c.inc b/target/ppc/translate/vsx-ops.c.inc > > index c974324c4c..67fa7b2e41 100644 > > --- a/target/ppc/translate/vsx-ops.c.inc > > +++ b/target/ppc/translate/vsx-ops.c.inc > > @@ -197,7 +197,6 @@ GEN_XX3FORM_NAME(xsnmsubdp, "xsnmsubmdp", 0x04, 0x17, PPC2_VSX), > > GEN_XX3FORM(xscmpeqdp, 0x0C, 0x00, PPC2_ISA300), > > GEN_XX3FORM(xscmpgtdp, 0x0C, 0x01, PPC2_ISA300), > > GEN_XX3FORM(xscmpgedp, 0x0C, 0x02, PPC2_ISA300), > > -GEN_XX3FORM(xscmpnedp, 0x0C, 0x03, PPC2_ISA300), > > GEN_XX3FORM(xscmpexpdp, 0x0C, 0x07, PPC2_ISA300), > > GEN_VSX_XFORM_300(xscmpexpqp, 0x04, 0x05, 0x00600001), > > GEN_XX2IFORM(xscmpodp, 0x0C, 0x05, PPC2_VSX), > > >
On 1/6/22 3:23 AM, Víctor Colombo wrote: > xscmpnedp was added in ISA v3.0 but removed in v3.0B. This patch > removes this instruction as it was not in the final version of v3.0. > > RFC to know if you think this is the correct approach. Should be fine. I assume the -cpu power9 that we intend to emulate is 3.0B? Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On Thu, Jan 06, 2022 at 02:02:01PM +0100, Greg Kurz wrote: > On Thu, 6 Jan 2022 13:21:46 +0100 > Cédric Le Goater <clg@kaod.org> wrote: > > > On 1/6/22 12:23, Víctor Colombo wrote: > > > xscmpnedp was added in ISA v3.0 but removed in v3.0B. This patch > > > removes this instruction as it was not in the final version of v3.0. > > > > > > RFC to know if you think this is the correct approach. > > > > Usually we deprecate a feature for a minimum of two releases before > > removing it. It might be overkill for this case since the P9 processor > > implementation is based on v3.0B. > > > > I would simply remove the instruction since it never existed on any > > supported HW. I will wait for some more feedback. > > > > I don't think it makes sense to keep this instruction if it only > existed in pre-GA HW. I agree. If we have a vistigial POWER9 DD1 in the cpu table we should probably remove that anyway. > Acked-by: Greg Kurz <groug@kaod.org> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > > > Reviewed-by: Cédric Le Goater <clg@kaod.org> > > > > Thanks, > > > > C. > > > > > > > Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br> > > > --- > > > target/ppc/fpu_helper.c | 1 - > > > target/ppc/helper.h | 1 - > > > target/ppc/translate/vsx-impl.c.inc | 1 - > > > target/ppc/translate/vsx-ops.c.inc | 1 - > > > 4 files changed, 4 deletions(-) > > > > > > diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c > > > index e5c29b53b8..f030858cf9 100644 > > > --- a/target/ppc/fpu_helper.c > > > +++ b/target/ppc/fpu_helper.c > > > @@ -2270,7 +2270,6 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, \ > > > VSX_SCALAR_CMP_DP(xscmpeqdp, eq, 1, 0) > > > VSX_SCALAR_CMP_DP(xscmpgedp, le, 1, 1) > > > VSX_SCALAR_CMP_DP(xscmpgtdp, lt, 1, 1) > > > -VSX_SCALAR_CMP_DP(xscmpnedp, eq, 0, 0) > > > > > > void helper_xscmpexpdp(CPUPPCState *env, uint32_t opcode, > > > ppc_vsr_t *xa, ppc_vsr_t *xb) > > > diff --git a/target/ppc/helper.h b/target/ppc/helper.h > > > index f9c72dcd50..8f02cabaf5 100644 > > > --- a/target/ppc/helper.h > > > +++ b/target/ppc/helper.h > > > @@ -400,7 +400,6 @@ DEF_HELPER_5(xsnmsubdp, void, env, vsr, vsr, vsr, vsr) > > > DEF_HELPER_4(xscmpeqdp, void, env, vsr, vsr, vsr) > > > DEF_HELPER_4(xscmpgtdp, void, env, vsr, vsr, vsr) > > > DEF_HELPER_4(xscmpgedp, void, env, vsr, vsr, vsr) > > > -DEF_HELPER_4(xscmpnedp, void, env, vsr, vsr, vsr) > > > DEF_HELPER_4(xscmpexpdp, void, env, i32, vsr, vsr) > > > DEF_HELPER_4(xscmpexpqp, void, env, i32, vsr, vsr) > > > DEF_HELPER_4(xscmpodp, void, env, i32, vsr, vsr) > > > diff --git a/target/ppc/translate/vsx-impl.c.inc b/target/ppc/translate/vsx-impl.c.inc > > > index c08185e857..fbef496257 100644 > > > --- a/target/ppc/translate/vsx-impl.c.inc > > > +++ b/target/ppc/translate/vsx-impl.c.inc > > > @@ -1092,7 +1092,6 @@ GEN_VSX_HELPER_X1(xstsqrtdp, 0x14, 0x06, 0, PPC2_VSX) > > > GEN_VSX_HELPER_X3(xscmpeqdp, 0x0C, 0x00, 0, PPC2_ISA300) > > > GEN_VSX_HELPER_X3(xscmpgtdp, 0x0C, 0x01, 0, PPC2_ISA300) > > > GEN_VSX_HELPER_X3(xscmpgedp, 0x0C, 0x02, 0, PPC2_ISA300) > > > -GEN_VSX_HELPER_X3(xscmpnedp, 0x0C, 0x03, 0, PPC2_ISA300) > > > GEN_VSX_HELPER_X2_AB(xscmpexpdp, 0x0C, 0x07, 0, PPC2_ISA300) > > > GEN_VSX_HELPER_R2_AB(xscmpexpqp, 0x04, 0x05, 0, PPC2_ISA300) > > > GEN_VSX_HELPER_X2_AB(xscmpodp, 0x0C, 0x05, 0, PPC2_VSX) > > > diff --git a/target/ppc/translate/vsx-ops.c.inc b/target/ppc/translate/vsx-ops.c.inc > > > index c974324c4c..67fa7b2e41 100644 > > > --- a/target/ppc/translate/vsx-ops.c.inc > > > +++ b/target/ppc/translate/vsx-ops.c.inc > > > @@ -197,7 +197,6 @@ GEN_XX3FORM_NAME(xsnmsubdp, "xsnmsubmdp", 0x04, 0x17, PPC2_VSX), > > > GEN_XX3FORM(xscmpeqdp, 0x0C, 0x00, PPC2_ISA300), > > > GEN_XX3FORM(xscmpgtdp, 0x0C, 0x01, PPC2_ISA300), > > > GEN_XX3FORM(xscmpgedp, 0x0C, 0x02, PPC2_ISA300), > > > -GEN_XX3FORM(xscmpnedp, 0x0C, 0x03, PPC2_ISA300), > > > GEN_XX3FORM(xscmpexpdp, 0x0C, 0x07, PPC2_ISA300), > > > GEN_VSX_XFORM_300(xscmpexpqp, 0x04, 0x05, 0x00600001), > > > GEN_XX2IFORM(xscmpodp, 0x0C, 0x05, PPC2_VSX), > > > > > >
On 1/6/22 12:23, Víctor Colombo wrote: > xscmpnedp was added in ISA v3.0 but removed in v3.0B. This patch > removes this instruction as it was not in the final version of v3.0. Could please resend on top of the VSX combo patchset ? Thanks, C.
On 10/01/2022 12:02, Cédric Le Goater wrote: > On 1/6/22 12:23, Víctor Colombo wrote: >> xscmpnedp was added in ISA v3.0 but removed in v3.0B. This patch >> removes this instruction as it was not in the final version of v3.0. > > Could please resend on top of the VSX combo patchset ? > Absolutely! I will add it between "Implement xvtlsbb instruction" and "Refactor VSX_SCALAR_CMP_DP" instead of on top, to avoid moving it to decodetree just to remove it later. We will send it together with the patchset v2 later. > Thanks, > > C. Thanks for everyone's time! Best regards, -- Víctor
On 1/10/22 18:57, Victor Colombo wrote: > > > On 10/01/2022 12:02, Cédric Le Goater wrote: >> On 1/6/22 12:23, Víctor Colombo wrote: >>> xscmpnedp was added in ISA v3.0 but removed in v3.0B. This patch >>> removes this instruction as it was not in the final version of v3.0. >> >> Could please resend on top of the VSX combo patchset ? >> > Absolutely! I will add it between "Implement xvtlsbb instruction" and "Refactor VSX_SCALAR_CMP_DP" instead of on top, to avoid moving it to decodetree just to remove it later. We will send it together with the patchset v2 later. Ah. if you resend the whole, perfect. It looks correct from the tests I did. And fix your name to Víctor ! Thanks, Cédric.
diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c index e5c29b53b8..f030858cf9 100644 --- a/target/ppc/fpu_helper.c +++ b/target/ppc/fpu_helper.c @@ -2270,7 +2270,6 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, \ VSX_SCALAR_CMP_DP(xscmpeqdp, eq, 1, 0) VSX_SCALAR_CMP_DP(xscmpgedp, le, 1, 1) VSX_SCALAR_CMP_DP(xscmpgtdp, lt, 1, 1) -VSX_SCALAR_CMP_DP(xscmpnedp, eq, 0, 0) void helper_xscmpexpdp(CPUPPCState *env, uint32_t opcode, ppc_vsr_t *xa, ppc_vsr_t *xb) diff --git a/target/ppc/helper.h b/target/ppc/helper.h index f9c72dcd50..8f02cabaf5 100644 --- a/target/ppc/helper.h +++ b/target/ppc/helper.h @@ -400,7 +400,6 @@ DEF_HELPER_5(xsnmsubdp, void, env, vsr, vsr, vsr, vsr) DEF_HELPER_4(xscmpeqdp, void, env, vsr, vsr, vsr) DEF_HELPER_4(xscmpgtdp, void, env, vsr, vsr, vsr) DEF_HELPER_4(xscmpgedp, void, env, vsr, vsr, vsr) -DEF_HELPER_4(xscmpnedp, void, env, vsr, vsr, vsr) DEF_HELPER_4(xscmpexpdp, void, env, i32, vsr, vsr) DEF_HELPER_4(xscmpexpqp, void, env, i32, vsr, vsr) DEF_HELPER_4(xscmpodp, void, env, i32, vsr, vsr) diff --git a/target/ppc/translate/vsx-impl.c.inc b/target/ppc/translate/vsx-impl.c.inc index c08185e857..fbef496257 100644 --- a/target/ppc/translate/vsx-impl.c.inc +++ b/target/ppc/translate/vsx-impl.c.inc @@ -1092,7 +1092,6 @@ GEN_VSX_HELPER_X1(xstsqrtdp, 0x14, 0x06, 0, PPC2_VSX) GEN_VSX_HELPER_X3(xscmpeqdp, 0x0C, 0x00, 0, PPC2_ISA300) GEN_VSX_HELPER_X3(xscmpgtdp, 0x0C, 0x01, 0, PPC2_ISA300) GEN_VSX_HELPER_X3(xscmpgedp, 0x0C, 0x02, 0, PPC2_ISA300) -GEN_VSX_HELPER_X3(xscmpnedp, 0x0C, 0x03, 0, PPC2_ISA300) GEN_VSX_HELPER_X2_AB(xscmpexpdp, 0x0C, 0x07, 0, PPC2_ISA300) GEN_VSX_HELPER_R2_AB(xscmpexpqp, 0x04, 0x05, 0, PPC2_ISA300) GEN_VSX_HELPER_X2_AB(xscmpodp, 0x0C, 0x05, 0, PPC2_VSX) diff --git a/target/ppc/translate/vsx-ops.c.inc b/target/ppc/translate/vsx-ops.c.inc index c974324c4c..67fa7b2e41 100644 --- a/target/ppc/translate/vsx-ops.c.inc +++ b/target/ppc/translate/vsx-ops.c.inc @@ -197,7 +197,6 @@ GEN_XX3FORM_NAME(xsnmsubdp, "xsnmsubmdp", 0x04, 0x17, PPC2_VSX), GEN_XX3FORM(xscmpeqdp, 0x0C, 0x00, PPC2_ISA300), GEN_XX3FORM(xscmpgtdp, 0x0C, 0x01, PPC2_ISA300), GEN_XX3FORM(xscmpgedp, 0x0C, 0x02, PPC2_ISA300), -GEN_XX3FORM(xscmpnedp, 0x0C, 0x03, PPC2_ISA300), GEN_XX3FORM(xscmpexpdp, 0x0C, 0x07, PPC2_ISA300), GEN_VSX_XFORM_300(xscmpexpqp, 0x04, 0x05, 0x00600001), GEN_XX2IFORM(xscmpodp, 0x0C, 0x05, PPC2_VSX),
xscmpnedp was added in ISA v3.0 but removed in v3.0B. This patch removes this instruction as it was not in the final version of v3.0. RFC to know if you think this is the correct approach. Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br> --- target/ppc/fpu_helper.c | 1 - target/ppc/helper.h | 1 - target/ppc/translate/vsx-impl.c.inc | 1 - target/ppc/translate/vsx-ops.c.inc | 1 - 4 files changed, 4 deletions(-)