| Message ID | 20231106132713.953501-7-adhemerval.zanella@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Multiple floating-point environment fixes | expand |
On 11/6/23 08:27, Adhemerval Zanella wrote: > From: Bruno Haible <bruno@clisp.org> > > It clears some exception flags that are outside the EXCEPTS argument. > > It fixes math/test-fexcept on qemu-user. > --- > sysdeps/alpha/fpu/fsetexcptflg.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/sysdeps/alpha/fpu/fsetexcptflg.c b/sysdeps/alpha/fpu/fsetexcptflg.c > index 70f3666a6e..63eb06845d 100644 > --- a/sysdeps/alpha/fpu/fsetexcptflg.c > +++ b/sysdeps/alpha/fpu/fsetexcptflg.c > @@ -27,7 +27,7 @@ __fesetexceptflag (const fexcept_t *flagp, int excepts) > tmp = __ieee_get_fp_control (); > > /* Set all the bits that were called for. */ > - tmp = (tmp & ~SWCR_STATUS_MASK) | (*flagp & excepts & SWCR_STATUS_MASK); > + tmp ^= (tmp ^ *flagp) & excepts & SWCR_STATUS_MASK; Does this actually work? Assume excepts is FE_INVALID, and *flagp bit 17 is 0. Assume currently bit 17 is 1. tmp ^ *flagp => bit 17 is still 1, even though bit 17 in flagp is 0. & excepts => bit 17 is still 1. & SWCR_STATUS_MASK => bit 17 is still 1. ^= => bit 17 is still 1. The operation will set a bit, but won't clear it? I would expect (taken from hppa code I wrote for that port): /* Clear all status bits we care about. */ tmp = tmp & ~(excepts & SWCR_STATUS_MASK); /* Install the new ones. */ tmp |= *flagp & excepts & SWCR_STATUS_MASK; > > /* And store it back. */ > __ieee_set_fp_control (tmp);
Carlos O'Donell wrote: > I would expect (taken from hppa code I wrote for that port): > > /* Clear all status bits we care about. */ > tmp = tmp & ~(excepts & SWCR_STATUS_MASK); > /* Install the new ones. */ > tmp |= *flagp & excepts & SWCR_STATUS_MASK; This code is correct. And, as I wrote in https://sourceware.org/bugzilla/show_bug.cgi?id=30998#c2 the code with the double-xor is faster, because it uses one less instruction. Basically the code you proposed masks the bits to clear and the bits to set separately. Whereas the code with the double-xor masks the bits to *change* - a single AND instead of two. > > /* Set all the bits that were called for. */ > > - tmp = (tmp & ~SWCR_STATUS_MASK) | (*flagp & excepts & SWCR_STATUS_MASK); > > + tmp ^= (tmp ^ *flagp) & excepts & SWCR_STATUS_MASK; > > Does this actually work? Yes: It computes the bits to *change*, then applies the mask, then applies the changes. For bits in the mask, it does tmp.bit[i] ^= tmp.bit[i] ^ (*flagp).bit[i]; which simplifies to tmp.bit[i] = (*flagp).bit[i]; For bits outside the mask, it does tmp.bit[i] ^= 0; i.e. it leaves the bit unchanged. I'm using this same idiom also in gnulib https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/fenv-except-state-set.c;h=2035f61e2714126e432c0c1f29450f18f58d81f1;hb=HEAD#l182 and I have verified that it works. > Assume excepts is FE_INVALID, and *flagp bit 17 is 0. > Assume currently bit 17 is 1. > > tmp ^ *flagp => bit 17 is still 1, even though bit 17 in flagp is 0. > & excepts => bit 17 is still 1. > & SWCR_STATUS_MASK => bit 17 is still 1. > ^= => bit 17 is still 1. In the last step: 1 xor 1 is 0. Bruno
On 11/6/23 12:36, Bruno Haible wrote: > I'm using this same idiom also in gnulib > https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/fenv-except-state-set.c;h=2035f61e2714126e432c0c1f29450f18f58d81f1;hb=HEAD#l182 > and I have verified that it works. That works for me. Thank you for verifying that. Reviewed-by: Carlos O'Donell <carlos@redhat.com>
diff --git a/sysdeps/alpha/fpu/fsetexcptflg.c b/sysdeps/alpha/fpu/fsetexcptflg.c index 70f3666a6e..63eb06845d 100644 --- a/sysdeps/alpha/fpu/fsetexcptflg.c +++ b/sysdeps/alpha/fpu/fsetexcptflg.c @@ -27,7 +27,7 @@ __fesetexceptflag (const fexcept_t *flagp, int excepts) tmp = __ieee_get_fp_control (); /* Set all the bits that were called for. */ - tmp = (tmp & ~SWCR_STATUS_MASK) | (*flagp & excepts & SWCR_STATUS_MASK); + tmp ^= (tmp ^ *flagp) & excepts & SWCR_STATUS_MASK; /* And store it back. */ __ieee_set_fp_control (tmp);
From: Bruno Haible <bruno@clisp.org> It clears some exception flags that are outside the EXCEPTS argument. It fixes math/test-fexcept on qemu-user. --- sysdeps/alpha/fpu/fsetexcptflg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)