Linux SNPS ARC Archive on lore.kernel.org
 help / color / Atom feed
From: Joseph Myers <joseph@codesourcery.com>
To: Vineet Gupta <Vineet.Gupta1@synopsys.com>
Cc: linux-snps-arc@lists.infradead.org, libc-alpha@sourceware.org
Subject: Re: [PATCH v4 07/15] ARC: hardware floating point support
Date: Thu, 26 Mar 2020 23:22:51 +0000
Message-ID: <alpine.DEB.2.21.2003262311330.24611@digraph.polyomino.org.uk> (raw)
In-Reply-To: <20200313030419.15843-8-vgupta@synopsys.com>

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@codesourcery.com

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

  parent reply index

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-13  3:04 [PATCH v4 00/15] glibc port to ARC processors Vineet Gupta
2020-03-13  3:04 ` [PATCH v4 01/15] ARC: add definitions to elf/elf.h Vineet Gupta
2020-03-26  1:37   ` Joseph Myers
2020-03-26  1:52     ` Vineet Gupta
2020-03-13  3:04 ` [PATCH v4 02/15] ARC: ABI Implementation Vineet Gupta
2020-03-26  1:52   ` Joseph Myers
2020-03-26  2:39     ` Vineet Gupta
2020-03-26 18:48       ` Joseph Myers
2020-03-27  0:37         ` Vineet Gupta
2020-03-27 23:33         ` Vineet Gupta
2020-03-27 23:35           ` Joseph Myers
2020-03-27 23:47             ` Vineet Gupta
2020-03-31 21:08         ` Big Endian support as multi-ABI (was Re: [PATCH v4 02/15] ARC: ABI Implementation) Vineet Gupta
2020-03-31 21:27           ` Joseph Myers
2020-03-31 21:35             ` Vineet Gupta
2020-03-13  3:04 ` [PATCH v4 03/15] ARC: startup and dynamic linking code Vineet Gupta
2020-03-26  1:55   ` Joseph Myers
2020-03-26  2:45     ` Vineet Gupta
2020-03-13  3:04 ` [PATCH v4 04/15] ARC: Thread Local Storage support Vineet Gupta
2020-03-26  1:57   ` Joseph Myers
2020-03-26  2:47     ` Vineet Gupta
2020-03-13  3:04 ` [PATCH v4 05/15] ARC: Atomics and Locking primitives Vineet Gupta
2020-03-13  3:04 ` [PATCH v4 06/15] ARC: math soft float support Vineet Gupta
2020-03-26  1:59   ` Joseph Myers
2020-03-26  2:48     ` Vineet Gupta
2020-03-13  3:04 ` [PATCH v4 07/15] ARC: hardware floating point support Vineet Gupta
2020-03-26  2:06   ` Joseph Myers
2020-03-26  3:19     ` Vineet Gupta
2020-03-26 23:22   ` Joseph Myers [this message]
2020-03-27  1:50     ` Vineet Gupta
2020-03-27 18:37       ` Joseph Myers
2020-03-27 18:53         ` Vineet Gupta
2020-03-13  3:04 ` [PATCH v4 08/15] ARC: Linux Syscall Interface Vineet Gupta
2020-03-26 23:52   ` Joseph Myers
2020-03-27  4:34     ` Vineet Gupta
2020-03-27 18:38       ` Joseph Myers
2020-03-13  3:04 ` [PATCH v4 09/15] ARC: Linux ABI Vineet Gupta
2020-03-27  0:38   ` Joseph Myers
2020-03-27  4:45     ` Vineet Gupta
2020-03-13  3:04 ` [PATCH v4 10/15] ARC: Linux Startup and Dynamic Loading Vineet Gupta
2020-03-13  3:04 ` [PATCH v4 11/15] ARC: ABI lists Vineet Gupta
2020-03-27  0:40   ` Joseph Myers
2020-03-27  4:36     ` Vineet Gupta
2020-03-27 18:39       ` Joseph Myers
2020-03-27 19:09         ` Vineet Gupta
2020-03-13  3:04 ` [PATCH v4 12/15] ARC: Update syscall-names.list for ARC specific syscalls Vineet Gupta
2020-03-13  3:04 ` [PATCH v4 13/15] ARC: Build Infrastructure Vineet Gupta
2020-03-27 22:47   ` Joseph Myers
2020-03-28  6:42     ` Vineet Gupta
2020-03-31 22:02       ` Vineet Gupta
2020-03-31 22:48         ` Joseph Myers
2020-04-01  0:44           ` __syscall_error (was Re: [PATCH v4 13/15] ARC: Build Infrastructure) Vineet Gupta
2020-04-01  7:58             ` Andreas Schwab
2020-04-01 21:38               ` Vineet Gupta
2020-04-01 17:06             ` Joseph Myers
2020-04-02  0:00               ` Vineet Gupta
2020-04-02  8:50               ` Florian Weimer
2020-04-02 20:22                 ` Vineet Gupta
2020-03-13  3:04 ` [PATCH v4 14/15] build-many-glibcs.py: Enable ARC builds Vineet Gupta
2020-03-13  3:04 ` [PATCH v4 15/15] Documentation for ARC port Vineet Gupta
2020-03-27 22:49   ` Joseph Myers
2020-03-27 23:56     ` Vineet Gupta
2020-03-28  0:01       ` Joseph Myers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.21.2003262311330.24611@digraph.polyomino.org.uk \
    --to=joseph@codesourcery.com \
    --cc=Vineet.Gupta1@synopsys.com \
    --cc=libc-alpha@sourceware.org \
    --cc=linux-snps-arc@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux SNPS ARC Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-snps-arc/0 linux-snps-arc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-snps-arc linux-snps-arc/ https://lore.kernel.org/linux-snps-arc \
		linux-snps-arc@lists.infradead.org
	public-inbox-index linux-snps-arc

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-snps-arc


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git