On Thu, 12 Mar 2020, Vineet Gupta via Libc-alpha wrote:
> +int
> +fegetmode (femode_t *modep)
> +{
> + unsigned int fpcr;
> +
> + _FPU_GETCW (fpcr);
> + *modep = fpcr >> __FPU_RND_SHIFT;
The bits to enable exception traps look like dynamic control mode bits to
me. In general fegetmode should only need to mask off bits on
architectures where the same register has both control and status bits,
not on architectures where those are separate registers and fegetmode /
fesetmode can work with the whole control register.
> +int
> +__fesetround (int round)
> +{
> + unsigned int fpcr;
> +
> + _FPU_GETCW (fpcr);
> +
> + if (__glibc_unlikely (((fpcr >> __FPU_RND_SHIFT) & FE_DOWNWARD) != round))
> + {
> + fpcr = (fpcr & ~(FE_DOWNWARD << __FPU_RND_SHIFT)) | (round << __FPU_RND_SHIFT);
> + _FPU_SETCW (fpcr);
> + }
I don't think the use of __glibc_unlikely is appropriate here. It's not
at all clear to me that the normal fesetround case is setting the rounding
mode to the value it already has, as the use of __glibc_unlikely would
suggest.
> +int
> +__feupdateenv (const fenv_t *envp)
> +{
> + unsigned int fpcr;
> + unsigned int fpsr;
> +
> + _FPU_GETCW (fpcr);
> + _FPU_GETS (fpsr);
> +
> + /* rounding mode set to what is in env. */
> + fpcr = envp->__fpcr;
> +
> + /* currently raised exceptions are OR'ed with env. */
> + fpsr |= envp->__fpsr;
This looks like it wouldn't work for FE_DFL_ENV, which is a valid argument
to feupdateenv. It looks like we're missing test coverage for feupdateenv
(FE_DFL_ENV) (we have coverage for feupdateenv (FE_NOMASK_ENV) and
fesetenv (FE_DFL_ENV)).
> +static inline int
> +get_rounding_mode (void)
> +{
> +#if defined(__ARC_FPU_SP__) || defined(__ARC_FPU_DP__)
> + unsigned int fpcr;
> + _FPU_GETCW (fpcr);
> +
> + return fpcr >> __FPU_RND_SHIFT;
Both here and in fegetround you're not doing anything to mask off high
bits of the control register. That seems unsafe to me, should future
processors add new control bits in the high bits that might sometimes be
nonzero.
--
Joseph S. Myers
joseph@xxxxxxxxxxxxxxxx
_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
