| Message ID | 1422559909-19377-1-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Thu, Jan 29, 2015 at 8:31 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > The helper functions for FRECPS and FRSQRTS have special case > handling that includes checks for zero inputs, so squash input > denormals if necessary before those checks. This fixes incorrect > output when the FPCR DZ bit is set to enable squashing of input > denormals. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Tested-by: Laurent Desnogues <laurent.desnogues@gmail.com> Thanks, Laurent > --- > A quick eyeball of helper-a64.c suggests that these are the only > other insns we needed to fix, and a risu test of these insns > confirms that (a) they're buggy and (b) this patch fixes them. > I haven't done an exhaustive coverage test of the whole instruction > set with the DZ bit set, though... > > target-arm/helper-a64.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c > index ebd9247..8aa40e9 100644 > --- a/target-arm/helper-a64.c > +++ b/target-arm/helper-a64.c > @@ -229,6 +229,9 @@ float32 HELPER(recpsf_f32)(float32 a, float32 b, void *fpstp) > { > float_status *fpst = fpstp; > > + a = float32_squash_input_denormal(a, fpst); > + b = float32_squash_input_denormal(b, fpst); > + > a = float32_chs(a); > if ((float32_is_infinity(a) && float32_is_zero(b)) || > (float32_is_infinity(b) && float32_is_zero(a))) { > @@ -241,6 +244,9 @@ float64 HELPER(recpsf_f64)(float64 a, float64 b, void *fpstp) > { > float_status *fpst = fpstp; > > + a = float64_squash_input_denormal(a, fpst); > + b = float64_squash_input_denormal(b, fpst); > + > a = float64_chs(a); > if ((float64_is_infinity(a) && float64_is_zero(b)) || > (float64_is_infinity(b) && float64_is_zero(a))) { > @@ -253,6 +259,9 @@ float32 HELPER(rsqrtsf_f32)(float32 a, float32 b, void *fpstp) > { > float_status *fpst = fpstp; > > + a = float32_squash_input_denormal(a, fpst); > + b = float32_squash_input_denormal(b, fpst); > + > a = float32_chs(a); > if ((float32_is_infinity(a) && float32_is_zero(b)) || > (float32_is_infinity(b) && float32_is_zero(a))) { > @@ -265,6 +274,9 @@ float64 HELPER(rsqrtsf_f64)(float64 a, float64 b, void *fpstp) > { > float_status *fpst = fpstp; > > + a = float64_squash_input_denormal(a, fpst); > + b = float64_squash_input_denormal(b, fpst); > + > a = float64_chs(a); > if ((float64_is_infinity(a) && float64_is_zero(b)) || > (float64_is_infinity(b) && float64_is_zero(a))) { > -- > 1.9.1 > >
On Fri, Jan 30, 2015 at 3:41 AM, Laurent Desnogues < laurent.desnogues@gmail.com> wrote: > On Thu, Jan 29, 2015 at 8:31 PM, Peter Maydell <peter.maydell@linaro.org> > wrote: > > The helper functions for FRECPS and FRSQRTS have special case > > handling that includes checks for zero inputs, so squash input > > denormals if necessary before those checks. This fixes incorrect > > output when the FPCR DZ bit is set to enable squashing of input > > denormals. > > > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > > Tested-by: Laurent Desnogues <laurent.desnogues@gmail.com> > > Thanks, > > Laurent > > > --- > > A quick eyeball of helper-a64.c suggests that these are the only > > other insns we needed to fix, and a risu test of these insns > > confirms that (a) they're buggy and (b) this patch fixes them. > > I haven't done an exhaustive coverage test of the whole instruction > > set with the DZ bit set, though... > > > > target-arm/helper-a64.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c > > index ebd9247..8aa40e9 100644 > > --- a/target-arm/helper-a64.c > > +++ b/target-arm/helper-a64.c > > @@ -229,6 +229,9 @@ float32 HELPER(recpsf_f32)(float32 a, float32 b, > void *fpstp) > > { > > float_status *fpst = fpstp; > > > > + a = float32_squash_input_denormal(a, fpst); > > + b = float32_squash_input_denormal(b, fpst); > > + > > a = float32_chs(a); > > if ((float32_is_infinity(a) && float32_is_zero(b)) || > > (float32_is_infinity(b) && float32_is_zero(a))) { > > @@ -241,6 +244,9 @@ float64 HELPER(recpsf_f64)(float64 a, float64 b, > void *fpstp) > > { > > float_status *fpst = fpstp; > > > > + a = float64_squash_input_denormal(a, fpst); > > + b = float64_squash_input_denormal(b, fpst); > > + > > a = float64_chs(a); > > if ((float64_is_infinity(a) && float64_is_zero(b)) || > > (float64_is_infinity(b) && float64_is_zero(a))) { > > @@ -253,6 +259,9 @@ float32 HELPER(rsqrtsf_f32)(float32 a, float32 b, > void *fpstp) > > { > > float_status *fpst = fpstp; > > > > + a = float32_squash_input_denormal(a, fpst); > > + b = float32_squash_input_denormal(b, fpst); > > + > > a = float32_chs(a); > > if ((float32_is_infinity(a) && float32_is_zero(b)) || > > (float32_is_infinity(b) && float32_is_zero(a))) { > > @@ -265,6 +274,9 @@ float64 HELPER(rsqrtsf_f64)(float64 a, float64 b, > void *fpstp) > > { > > float_status *fpst = fpstp; > > > > + a = float64_squash_input_denormal(a, fpst); > > + b = float64_squash_input_denormal(b, fpst); > > + > > a = float64_chs(a); > > if ((float64_is_infinity(a) && float64_is_zero(b)) || > > (float64_is_infinity(b) && float64_is_zero(a))) { > > -- > > 1.9.1 > > > > > > Reviewed-by: Greg Bellows <greg.bellows@linaro.org>
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c index ebd9247..8aa40e9 100644 --- a/target-arm/helper-a64.c +++ b/target-arm/helper-a64.c @@ -229,6 +229,9 @@ float32 HELPER(recpsf_f32)(float32 a, float32 b, void *fpstp) { float_status *fpst = fpstp; + a = float32_squash_input_denormal(a, fpst); + b = float32_squash_input_denormal(b, fpst); + a = float32_chs(a); if ((float32_is_infinity(a) && float32_is_zero(b)) || (float32_is_infinity(b) && float32_is_zero(a))) { @@ -241,6 +244,9 @@ float64 HELPER(recpsf_f64)(float64 a, float64 b, void *fpstp) { float_status *fpst = fpstp; + a = float64_squash_input_denormal(a, fpst); + b = float64_squash_input_denormal(b, fpst); + a = float64_chs(a); if ((float64_is_infinity(a) && float64_is_zero(b)) || (float64_is_infinity(b) && float64_is_zero(a))) { @@ -253,6 +259,9 @@ float32 HELPER(rsqrtsf_f32)(float32 a, float32 b, void *fpstp) { float_status *fpst = fpstp; + a = float32_squash_input_denormal(a, fpst); + b = float32_squash_input_denormal(b, fpst); + a = float32_chs(a); if ((float32_is_infinity(a) && float32_is_zero(b)) || (float32_is_infinity(b) && float32_is_zero(a))) { @@ -265,6 +274,9 @@ float64 HELPER(rsqrtsf_f64)(float64 a, float64 b, void *fpstp) { float_status *fpst = fpstp; + a = float64_squash_input_denormal(a, fpst); + b = float64_squash_input_denormal(b, fpst); + a = float64_chs(a); if ((float64_is_infinity(a) && float64_is_zero(b)) || (float64_is_infinity(b) && float64_is_zero(a))) {
The helper functions for FRECPS and FRSQRTS have special case handling that includes checks for zero inputs, so squash input denormals if necessary before those checks. This fixes incorrect output when the FPCR DZ bit is set to enable squashing of input denormals. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- A quick eyeball of helper-a64.c suggests that these are the only other insns we needed to fix, and a risu test of these insns confirms that (a) they're buggy and (b) this patch fixes them. I haven't done an exhaustive coverage test of the whole instruction set with the DZ bit set, though... target-arm/helper-a64.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)