| Message ID | 1469602609-31349-11-git-send-email-benh@kernel.crashing.org |
|---|---|
| State | New |
| Headers | show |
On Wed, Jul 27, 2016 at 04:56:29PM +1000, Benjamin Herrenschmidt wrote: > We don't implement imprecise FP exceptions and using store_current > which sets SRR1 to the *previous* instruction never makes sense > for these. So let's be truthful and make them precise, which is > allowed by the architecture. > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > --- > target-ppc/excp_helper.c | 11 ++++++----- > target-ppc/translate.c | 1 - > 2 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c > index 96c6fd9..02d9e79 100644 > --- a/target-ppc/excp_helper.c > +++ b/target-ppc/excp_helper.c > @@ -274,12 +274,13 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) > env->error_code = 0; > return; > } > + > + /* FP exceptions always have NIP pointing to the faulting > + * instruction, so always use store_next and claim we are > + * precise in the MSR. > + */ > msr |= 0x00100000; > - if (msr_fe0 == msr_fe1) { > - goto store_next; > - } > - msr |= 0x00010000; > - break; > + goto store_next; > case POWERPC_EXCP_INVAL: > LOG_EXCP("Invalid instruction at " TARGET_FMT_lx "\n", env->nip); > msr |= 0x00080000; > diff --git a/target-ppc/translate.c b/target-ppc/translate.c > index 3cfa40f..ba14bda 100644 > --- a/target-ppc/translate.c > +++ b/target-ppc/translate.c > @@ -3060,7 +3060,6 @@ static void gen_conditional_store(DisasContext *ctx, TCGv EA, > int reg, int size) > { > TCGv t0 = tcg_temp_new(); > - uint32_t save_exception = ctx->exception; This looks like an unrelated change, and one which would break compile without other changes in gen_conditional_store() that I don't see. Have you compiled the user-only targets with this change? > tcg_gen_st_tl(EA, cpu_env, offsetof(CPUPPCState, reserve_ea)); > tcg_gen_movi_tl(t0, (size << 5) | reg);
On Wed, 2016-07-27 at 17:21 +1000, David Gibson wrote: > On Wed, Jul 27, 2016 at 04:56:29PM +1000, Benjamin Herrenschmidt > wrote: > > > > We don't implement imprecise FP exceptions and using store_current > > which sets SRR1 to the *previous* instruction never makes sense > > for these. So let's be truthful and make them precise, which is > > allowed by the architecture. > > > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > --- > > target-ppc/excp_helper.c | 11 ++++++----- > > target-ppc/translate.c | 1 - > > 2 files changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c > > index 96c6fd9..02d9e79 100644 > > --- a/target-ppc/excp_helper.c > > +++ b/target-ppc/excp_helper.c > > @@ -274,12 +274,13 @@ static inline void powerpc_excp(PowerPCCPU > > *cpu, int excp_model, int excp) > > env->error_code = 0; > > return; > > } > > + > > + /* FP exceptions always have NIP pointing to the > > faulting > > + * instruction, so always use store_next and claim we > > are > > + * precise in the MSR. > > + */ > > msr |= 0x00100000; > > - if (msr_fe0 == msr_fe1) { > > - goto store_next; > > - } > > - msr |= 0x00010000; > > - break; > > + goto store_next; > > case POWERPC_EXCP_INVAL: > > LOG_EXCP("Invalid instruction at " TARGET_FMT_lx "\n", > > env->nip); > > msr |= 0x00080000; > > diff --git a/target-ppc/translate.c b/target-ppc/translate.c > > index 3cfa40f..ba14bda 100644 > > --- a/target-ppc/translate.c > > +++ b/target-ppc/translate.c > > @@ -3060,7 +3060,6 @@ static void > > gen_conditional_store(DisasContext *ctx, TCGv EA, > > int reg, int size) > > { > > TCGv t0 = tcg_temp_new(); > > - uint32_t save_exception = ctx->exception; > > This looks like an unrelated change, and one which would break > compile > without other changes in gen_conditional_store() that I don't see. > > Have you compiled the user-only targets with this change? Ah yes, the whole series does work in user-only, I tested, it's just a messup beween 2 commits. The rest of the change is in: "ppc: Rework NIP updates vs. exception generation" What happened is that I initially forgot to remove that line from the above commit, causing a warning. I did a fix, then folded the fix into the wrong commmit :) You can either take it out of this commit and put it into the above mentioned one, or I can do a 3rd spin... but the fix is trivial. Cheers, Ben. > > > > tcg_gen_st_tl(EA, cpu_env, offsetof(CPUPPCState, reserve_ea)); > > tcg_gen_movi_tl(t0, (size << 5) | reg); >
On Wed, Jul 27, 2016 at 07:44:26PM +1000, Benjamin Herrenschmidt wrote: > On Wed, 2016-07-27 at 17:21 +1000, David Gibson wrote: > > On Wed, Jul 27, 2016 at 04:56:29PM +1000, Benjamin Herrenschmidt > > wrote: > > > > > > We don't implement imprecise FP exceptions and using store_current > > > which sets SRR1 to the *previous* instruction never makes sense > > > for these. So let's be truthful and make them precise, which is > > > allowed by the architecture. > > > > > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > > --- > > > target-ppc/excp_helper.c | 11 ++++++----- > > > target-ppc/translate.c | 1 - > > > 2 files changed, 6 insertions(+), 6 deletions(-) > > > > > > diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c > > > index 96c6fd9..02d9e79 100644 > > > --- a/target-ppc/excp_helper.c > > > +++ b/target-ppc/excp_helper.c > > > @@ -274,12 +274,13 @@ static inline void powerpc_excp(PowerPCCPU > > > *cpu, int excp_model, int excp) > > > env->error_code = 0; > > > return; > > > } > > > + > > > + /* FP exceptions always have NIP pointing to the > > > faulting > > > + * instruction, so always use store_next and claim we > > > are > > > + * precise in the MSR. > > > + */ > > > msr |= 0x00100000; > > > - if (msr_fe0 == msr_fe1) { > > > - goto store_next; > > > - } > > > - msr |= 0x00010000; > > > - break; > > > + goto store_next; > > > case POWERPC_EXCP_INVAL: > > > LOG_EXCP("Invalid instruction at " TARGET_FMT_lx "\n", > > > env->nip); > > > msr |= 0x00080000; > > > diff --git a/target-ppc/translate.c b/target-ppc/translate.c > > > index 3cfa40f..ba14bda 100644 > > > --- a/target-ppc/translate.c > > > +++ b/target-ppc/translate.c > > > @@ -3060,7 +3060,6 @@ static void > > > gen_conditional_store(DisasContext *ctx, TCGv EA, > > > int reg, int size) > > > { > > > TCGv t0 = tcg_temp_new(); > > > - uint32_t save_exception = ctx->exception; > > > > This looks like an unrelated change, and one which would break > > compile > > without other changes in gen_conditional_store() that I don't see. > > > > Have you compiled the user-only targets with this change? > > Ah yes, the whole series does work in user-only, I tested, it's > just a messup beween 2 commits. The rest of the change is in: > > "ppc: Rework NIP updates vs. exception generation" > > What happened is that I initially forgot to remove that line > from the above commit, causing a warning. I did a fix, then > folded the fix into the wrong commmit :) > > You can either take it out of this commit and put it into the > above mentioned one, or I can do a 3rd spin... but the fix is > trivial. Ok, I've fixed it up in place.
diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c index 96c6fd9..02d9e79 100644 --- a/target-ppc/excp_helper.c +++ b/target-ppc/excp_helper.c @@ -274,12 +274,13 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) env->error_code = 0; return; } + + /* FP exceptions always have NIP pointing to the faulting + * instruction, so always use store_next and claim we are + * precise in the MSR. + */ msr |= 0x00100000; - if (msr_fe0 == msr_fe1) { - goto store_next; - } - msr |= 0x00010000; - break; + goto store_next; case POWERPC_EXCP_INVAL: LOG_EXCP("Invalid instruction at " TARGET_FMT_lx "\n", env->nip); msr |= 0x00080000; diff --git a/target-ppc/translate.c b/target-ppc/translate.c index 3cfa40f..ba14bda 100644 --- a/target-ppc/translate.c +++ b/target-ppc/translate.c @@ -3060,7 +3060,6 @@ static void gen_conditional_store(DisasContext *ctx, TCGv EA, int reg, int size) { TCGv t0 = tcg_temp_new(); - uint32_t save_exception = ctx->exception; tcg_gen_st_tl(EA, cpu_env, offsetof(CPUPPCState, reserve_ea)); tcg_gen_movi_tl(t0, (size << 5) | reg);
We don't implement imprecise FP exceptions and using store_current which sets SRR1 to the *previous* instruction never makes sense for these. So let's be truthful and make them precise, which is allowed by the architecture. Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> --- target-ppc/excp_helper.c | 11 ++++++----- target-ppc/translate.c | 1 - 2 files changed, 6 insertions(+), 6 deletions(-)