public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] arc: Add --with-fpu support for ARCv2 cpus
@ 2021-06-04  7:29 Claudiu Zissulescu
  2021-06-04  9:04 ` Bernhard Reutner-Fischer
  2021-06-05  5:46 ` Jeff Law
  0 siblings, 2 replies; 12+ messages in thread
From: Claudiu Zissulescu @ 2021-06-04  7:29 UTC (permalink / raw)
  To: gcc-patches; +Cc: fbedard, law

Hi Jeff,

I would like to add spport for selecting the ARCv2 FPU extension at
configuration-time.

The --with-fpu configuration option is ignored when -mfpu compiler
option is specified.

My concern is using `grep -P` when configuring. Is that ok?

Thanks,
Claudiu

gcc/
yyyy-mm-dd  Claudiu Zissulescu  <claziss@synopsys.com>

	* config.gcc (arc): Add support for with_cpu option.
	* config/arc/arc.h (OPTION_DEFAULT_SPECS): Add fpu.

Signed-off-by: Claudiu Zissulescu <claziss@synopsys.com>
---
 gcc/config.gcc       | 56 ++++++++++++++++++++++++++++++++++++++++++--
 gcc/config/arc/arc.h |  4 ++++
 2 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/gcc/config.gcc b/gcc/config.gcc
index 610422fb29ee..f46b5e79af69 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -4258,18 +4258,70 @@ case "${target}" in
 		;;
 
 	arc*-*-*)
-		supported_defaults="cpu"
+		supported_defaults="cpu fpu"
 
+		new_cpu=hs38_linux
 		if [ x"$with_cpu" = x ] \
 		    || grep "^ARC_CPU ($with_cpu," \
 		       ${srcdir}/config/arc/arc-cpus.def \
 		       > /dev/null; then
 		 # Ok
-		 true
+		 new_cpu=$with_cpu
 		else
 		 echo "Unknown cpu used in --with-cpu=$with_cpu" 1>&2
 		 exit 1
 		fi
+
+		# see if --with-fpu matches any of the supported FPUs
+		case "$with_fpu" in
+		"")
+			# OK
+			;;
+		fpus | fpus_div | fpus_fma | fpus_all)
+			# OK if em or hs
+			if grep -P "^ARC_CPU \($new_cpu,\s+[emhs]+," \
+			   ${srcdir}/config/arc/arc-cpus.def \
+			   > /dev/null; then
+			   # OK
+			   true
+			else
+			 echo "Unknown floating point type used in "\
+			 "--with-fpu=$with_fpu for cpu $new_cpu" 1>&2
+			 exit 1
+			fi
+		;;
+		fpuda | fpuda_div | fpuda_fma | fpuda_all)
+			# OK only em
+			if grep -P "^ARC_CPU \($new_cpu,\s+em," \
+			   ${srcdir}/config/arc/arc-cpus.def \
+			   > /dev/null; then
+			   # OK
+			   true
+			else
+			 echo "Unknown floating point type used in "\
+			      "--with-fpu=$with_fpu for cpu $new_cpu" 1>&2
+			 exit 1
+			fi
+			;;
+		fpud | fpud_div | fpud_fma | fpud_all)
+			# OK only hs
+			if grep -P "^ARC_CPU \($new_cpu,\s+hs," \
+			   ${srcdir}/config/arc/arc-cpus.def \
+			   > /dev/null; then
+			   # OK
+			   true
+			else
+			 echo "Unknown floating point type used in"\
+			      "--with-fpu=$with_fpu for cpu $new_cpu" 1>&2
+			 exit 1
+			fi
+			;;
+		*)
+			echo "Unknown floating point type used in "\
+			     "--with-fpu=$with_fpu" 1>&2
+			exit 1
+			;;
+		esac
 		;;
 
     csky-*-*)
diff --git a/gcc/config/arc/arc.h b/gcc/config/arc/arc.h
index 722bb10b8813..b9c4ba0398e5 100644
--- a/gcc/config/arc/arc.h
+++ b/gcc/config/arc/arc.h
@@ -100,7 +100,11 @@ extern const char *arc_cpu_to_as (int argc, const char **argv);
   "%:cpu_to_as(%{mcpu=*:%*}) %{mspfp*} %{mdpfp*} "                      \
   "%{mfpu=fpuda*:-mfpuda} %{mcode-density}"
 
+/* Support for a compile-time default CPU and FPU.  The rules are:
+   --with-cpu is ignored if -mcpu, mARC*, marc*, mA7, mA6 are specified.
+   --with-fpu is ignored if -mfpu is specified.  */
 #define OPTION_DEFAULT_SPECS						\
+  {"fpu", "%{!mfpu=*:-mfpu=%(VALUE)}"},					\
   {"cpu", "%{!mcpu=*:%{!mARC*:%{!marc*:%{!mA7:%{!mA6:-mcpu=%(VALUE)}}}}}" }
 
 #ifndef DRIVER_ENDIAN_SELF_SPECS
-- 
2.31.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arc: Add --with-fpu support for ARCv2 cpus
  2021-06-04  7:29 [PATCH] arc: Add --with-fpu support for ARCv2 cpus Claudiu Zissulescu
@ 2021-06-04  9:04 ` Bernhard Reutner-Fischer
  2021-06-05  5:46 ` Jeff Law
  1 sibling, 0 replies; 12+ messages in thread
From: Bernhard Reutner-Fischer @ 2021-06-04  9:04 UTC (permalink / raw)
  To: Claudiu Zissulescu
  Cc: rep.dot.nop, Claudiu Zissulescu via Gcc-patches, fbedard, law

On Fri,  4 Jun 2021 10:29:09 +0300
Claudiu Zissulescu via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:

> Hi Jeff,
> 
> I would like to add spport for selecting the ARCv2 FPU extension at
> configuration-time.
> 
> The --with-fpu configuration option is ignored when -mfpu compiler
> option is specified.
> 
> My concern is using `grep -P` when configuring. Is that ok?

Please don't.
Not every grep(1) has PCRE support so it would be great if you could
rephrase it to use just use normal regexp or ERE.

like e.g.:
grep -q -E "^ARC_CPU \(hs38,[ 	]*[emhs]+," config/arc/arc-cpus.def

where the space is <space><^vi>, i.e. space, tab
Or use an awk script that exits appropriately if the arg matches ARCH.

TIA,

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arc: Add --with-fpu support for ARCv2 cpus
  2021-06-04  7:29 [PATCH] arc: Add --with-fpu support for ARCv2 cpus Claudiu Zissulescu
  2021-06-04  9:04 ` Bernhard Reutner-Fischer
@ 2021-06-05  5:46 ` Jeff Law
  2021-06-08  7:05   ` Claudiu Zissulescu
  1 sibling, 1 reply; 12+ messages in thread
From: Jeff Law @ 2021-06-05  5:46 UTC (permalink / raw)
  To: Claudiu Zissulescu, gcc-patches; +Cc: fbedard, law



On 6/4/2021 1:29 AM, Claudiu Zissulescu via Gcc-patches wrote:
> Hi Jeff,
>
> I would like to add spport for selecting the ARCv2 FPU extension at
> configuration-time.
>
> The --with-fpu configuration option is ignored when -mfpu compiler
> option is specified.
>
> My concern is using `grep -P` when configuring. Is that ok?
>
> Thanks,
> Claudiu
>
> gcc/
> yyyy-mm-dd  Claudiu Zissulescu  <claziss@synopsys.com>
>
> 	* config.gcc (arc): Add support for with_cpu option.
> 	* config/arc/arc.h (OPTION_DEFAULT_SPECS): Add fpu.
I strongly suspect -P is a GNU-ism and probably won't work on other 
hosts were GCC is still used.  I'd avoid it and look for an alternate 
solution.

jeff


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arc: Add --with-fpu support for ARCv2 cpus
  2021-06-05  5:46 ` Jeff Law
@ 2021-06-08  7:05   ` Claudiu Zissulescu
  2021-06-08 10:19     ` Bernhard Reutner-Fischer
  0 siblings, 1 reply; 12+ messages in thread
From: Claudiu Zissulescu @ 2021-06-08  7:05 UTC (permalink / raw)
  To: Jeff Law, gcc-patches, rep.dot.nop; +Cc: fbedard

[-- Attachment #1: Type: text/plain, Size: 110 bytes --]

Thank you for your input.

I have made an update using grep's ERE. Please let me know if it is ok.

//Claudiu

[-- Attachment #2: 0001-arc-Add-with-fpu-support-for-ARCv2-cpus.patch --]
[-- Type: text/x-patch, Size: 3417 bytes --]

From 3f598e0fc9bc88c3f40f3e381c2955ab36e77ce0 Mon Sep 17 00:00:00 2001
From: Claudiu Zissulescu <claziss@synopsys.com>
Date: Wed, 21 Oct 2020 16:11:43 +0300
Subject: [PATCH] arc: Add --with-fpu support for ARCv2 cpus

Support for a compile-time default FPU. The --with-fpu configuration
option is ignored if -mfpu compiler option is specified. The FPU
options are only available for ARCv2 cpus.

gcc/
yyyy-mm-dd  Claudiu Zissulescu  <claziss@synopsys.com>

	* config.gcc (arc): Add support for with_cpu option.
	* config/arc/arc.h (OPTION_DEFAULT_SPECS): Add fpu.

Signed-off-by: Claudiu Zissulescu <claziss@synopsys.com>
---
 gcc/config.gcc       | 58 +++++++++++++++++++++++++++++++++++++++++---
 gcc/config/arc/arc.h |  4 +++
 2 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/gcc/config.gcc b/gcc/config.gcc
index 610422fb29ee..d4445e98e0c9 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -4258,18 +4258,70 @@ case "${target}" in
 		;;
 
 	arc*-*-*)
-		supported_defaults="cpu"
+		supported_defaults="cpu fpu"
 
+		new_cpu=hs38_linux
 		if [ x"$with_cpu" = x ] \
-		    || grep "^ARC_CPU ($with_cpu," \
+		    || grep -E "^ARC_CPU \($with_cpu," \
 		       ${srcdir}/config/arc/arc-cpus.def \
 		       > /dev/null; then
 		 # Ok
-		 true
+		 new_cpu=$with_cpu
 		else
 		 echo "Unknown cpu used in --with-cpu=$with_cpu" 1>&2
 		 exit 1
 		fi
+
+		# see if --with-fpu matches any of the supported FPUs
+		case "$with_fpu" in
+		"")
+			# OK
+			;;
+		fpus | fpus_div | fpus_fma | fpus_all)
+			# OK if em or hs
+			if grep -E "^ARC_CPU \($new_cpu,[[:space:]]+[emhs]+," \
+			   ${srcdir}/config/arc/arc-cpus.def \
+			   > /dev/null; then
+			   # OK
+			   true
+			else
+			 echo "Unknown floating point type used in "\
+			 "--with-fpu=$with_fpu for cpu $new_cpu" 1>&2
+			 exit 1
+			fi
+		;;
+		fpuda | fpuda_div | fpuda_fma | fpuda_all)
+			# OK only em
+			if grep -E "^ARC_CPU \($new_cpu,[[:space:]]+em," \
+			   ${srcdir}/config/arc/arc-cpus.def \
+			   > /dev/null; then
+			   # OK
+			   true
+			else
+			 echo "Unknown floating point type used in "\
+			      "--with-fpu=$with_fpu for cpu $new_cpu" 1>&2
+			 exit 1
+			fi
+			;;
+		fpud | fpud_div | fpud_fma | fpud_all)
+			# OK only hs
+			if grep -E "^ARC_CPU \($new_cpu,[[:space:]]+hs," \
+			   ${srcdir}/config/arc/arc-cpus.def \
+			   > /dev/null; then
+			   # OK
+			   true
+			else
+			 echo "Unknown floating point type used in"\
+			      "--with-fpu=$with_fpu for cpu $new_cpu" 1>&2
+			 exit 1
+			fi
+			;;
+		*)
+			echo "Unknown floating point type used in "\
+			     "--with-fpu=$with_fpu" 1>&2
+			exit 1
+			;;
+		esac
 		;;
 
     csky-*-*)
diff --git a/gcc/config/arc/arc.h b/gcc/config/arc/arc.h
index 722bb10b8813..b9c4ba0398e5 100644
--- a/gcc/config/arc/arc.h
+++ b/gcc/config/arc/arc.h
@@ -100,7 +100,11 @@ extern const char *arc_cpu_to_as (int argc, const char **argv);
   "%:cpu_to_as(%{mcpu=*:%*}) %{mspfp*} %{mdpfp*} "                      \
   "%{mfpu=fpuda*:-mfpuda} %{mcode-density}"
 
+/* Support for a compile-time default CPU and FPU.  The rules are:
+   --with-cpu is ignored if -mcpu, mARC*, marc*, mA7, mA6 are specified.
+   --with-fpu is ignored if -mfpu is specified.  */
 #define OPTION_DEFAULT_SPECS						\
+  {"fpu", "%{!mfpu=*:-mfpu=%(VALUE)}"},					\
   {"cpu", "%{!mcpu=*:%{!mARC*:%{!marc*:%{!mA7:%{!mA6:-mcpu=%(VALUE)}}}}}" }
 
 #ifndef DRIVER_ENDIAN_SELF_SPECS
-- 
2.31.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arc: Add --with-fpu support for ARCv2 cpus
  2021-06-08  7:05   ` Claudiu Zissulescu
@ 2021-06-08 10:19     ` Bernhard Reutner-Fischer
  2021-06-09 11:35       ` Claudiu Zissulescu
  0 siblings, 1 reply; 12+ messages in thread
From: Bernhard Reutner-Fischer @ 2021-06-08 10:19 UTC (permalink / raw)
  To: Claudiu Zissulescu; +Cc: rep.dot.nop, Jeff Law, gcc-patches, fbedard

On Tue, 8 Jun 2021 10:05:28 +0300
Claudiu Zissulescu <claziss@gmail.com> wrote:

> Thank you for your input.
> 
> I have made an update using grep's ERE. Please let me know if it is ok.

I would have written [[:space:]]* instead of [[:space:]]+ to handle
potentially missing space, at least after the comma but also before the
comma to avoid surprises for new names in the future.
Furthermore <space>|<tab> alone would be [[:blank:]]* but as you prefer.

grep ... > /dev/null would be grep -q which is mandated by POSIX since
at least SUSv2 so can be used safely since quite some time now.

Instead of the redundant 'true' calls, i'd usually write :
E.g.
if grep -q ... ; then :
else echo "nah"; exit 1
fi

Which could be shortened to
if ! grep -q ...
then
  echo "nah"
  exit 1
fi

to avoid any questions about an empty arm in the first place.

ISTM you only set the expected flags in the switch so i would have
set only that variable and have grepped only once after the switch for
brevity.

Either way, thanks for not using grep -P :)
thanks,

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arc: Add --with-fpu support for ARCv2 cpus
  2021-06-08 10:19     ` Bernhard Reutner-Fischer
@ 2021-06-09 11:35       ` Claudiu Zissulescu
  2021-06-09 12:26         ` Bernhard Reutner-Fischer
  0 siblings, 1 reply; 12+ messages in thread
From: Claudiu Zissulescu @ 2021-06-09 11:35 UTC (permalink / raw)
  To: Bernhard Reutner-Fischer; +Cc: Jeff Law, gcc-patches, fbedard

[-- Attachment #1: Type: text/plain, Size: 1283 bytes --]

Hi,

> I would have written [[:space:]]* instead of [[:space:]]+ to handle
> potentially missing space, at least after the comma but also before the
> comma to avoid surprises for new names in the future.
> Furthermore <space>|<tab> alone would be [[:blank:]]* but as you prefer.
> 
> grep ... > /dev/null would be grep -q which is mandated by POSIX since
> at least SUSv2 so can be used safely since quite some time now.
> 
> Instead of the redundant 'true' calls, i'd usually write :
> E.g.
> if grep -q ... ; then :
> else echo "nah"; exit 1
> fi
> 
> Which could be shortened to
> if ! grep -q ...
> then
>    echo "nah"
>    exit 1
> fi
> 
> to avoid any questions about an empty arm in the first place.

I've updated the patch using your feedback (attached). Indeed, it looks 
much better now :)

> 
> ISTM you only set the expected flags in the switch so i would have
> set only that variable and have grepped only once after the switch for
> brevity.

ARC has various FPU extensions, some of them are common to EM and HS 
architectures, others are specific for only one of them. Hence, the grep 
commands are ensuring that we accept the right fpu extension for the 
right ARC architecture.

> 
> Either way, thanks for not using grep -P :)
> thanks,
> 

I thank you!
Claudiu

[-- Attachment #2: 0001-arc-Add-with-fpu-support-for-ARCv2-cpus.patch --]
[-- Type: text/x-patch, Size: 3313 bytes --]

From 1f895d277752277fb51e8436903a94949bd5c7bd Mon Sep 17 00:00:00 2001
From: Claudiu Zissulescu <claziss@synopsys.com>
Date: Wed, 21 Oct 2020 16:11:43 +0300
Subject: [PATCH] arc: Add --with-fpu support for ARCv2 cpus

Support for a compile-time default FPU. The --with-fpu configuration
option is ignored if -mfpu compiler option is specified. The FPU
options are only available for ARCv2 cpus.

gcc/
yyyy-mm-dd  Claudiu Zissulescu  <claziss@synopsys.com>

	* config.gcc (arc): Add support for with_cpu option.
	* config/arc/arc.h (OPTION_DEFAULT_SPECS): Add fpu.

Signed-off-by: Claudiu Zissulescu <claziss@synopsys.com>
---
 gcc/config.gcc       | 49 +++++++++++++++++++++++++++++++++++++++++---
 gcc/config/arc/arc.h |  4 ++++
 2 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/gcc/config.gcc b/gcc/config.gcc
index 13c2004e3c52..09886c8635e0 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -4258,18 +4258,61 @@ case "${target}" in
 		;;
 
 	arc*-*-*)
-		supported_defaults="cpu"
+		supported_defaults="cpu fpu"
 
+		new_cpu=hs38_linux
 		if [ x"$with_cpu" = x ] \
-		    || grep "^ARC_CPU ($with_cpu," \
+		    || grep -E "^ARC_CPU \($with_cpu," \
 		       ${srcdir}/config/arc/arc-cpus.def \
 		       > /dev/null; then
 		 # Ok
-		 true
+		 new_cpu=$with_cpu
 		else
 		 echo "Unknown cpu used in --with-cpu=$with_cpu" 1>&2
 		 exit 1
 		fi
+
+		# see if --with-fpu matches any of the supported FPUs
+		case "$with_fpu" in
+		"")
+			# OK
+			;;
+		fpus | fpus_div | fpus_fma | fpus_all)
+			# OK if em or hs
+			if ! grep -q -E "^ARC_CPU[[:blank:]]*\($new_cpu,[[:space:]]*[emhs]+," \
+			   ${srcdir}/config/arc/arc-cpus.def
+			then
+			 echo "Unknown floating point type used in "\
+			 "--with-fpu=$with_fpu for cpu $new_cpu" 1>&2
+			 exit 1
+			fi
+		;;
+		fpuda | fpuda_div | fpuda_fma | fpuda_all)
+			# OK only em
+			if ! grep -q -E "^ARC_CPU[[:blank:]]*\($new_cpu,[[:space:]]*em," \
+			   ${srcdir}/config/arc/arc-cpus.def
+			then
+			 echo "Unknown floating point type used in "\
+			      "--with-fpu=$with_fpu for cpu $new_cpu" 1>&2
+			 exit 1
+			fi
+			;;
+		fpud | fpud_div | fpud_fma | fpud_all)
+			# OK only hs
+			if ! grep -q -E "^ARC_CPU[[:blank:]]*\($new_cpu,[[:space:]]*hs," \
+			   ${srcdir}/config/arc/arc-cpus.def
+			then
+			 echo "Unknown floating point type used in"\
+			      "--with-fpu=$with_fpu for cpu $new_cpu" 1>&2
+			 exit 1
+			fi
+			;;
+		*)
+			echo "Unknown floating point type used in "\
+			     "--with-fpu=$with_fpu" 1>&2
+			exit 1
+			;;
+		esac
 		;;
 
     csky-*-*)
diff --git a/gcc/config/arc/arc.h b/gcc/config/arc/arc.h
index 722bb10b8813..b9c4ba0398e5 100644
--- a/gcc/config/arc/arc.h
+++ b/gcc/config/arc/arc.h
@@ -100,7 +100,11 @@ extern const char *arc_cpu_to_as (int argc, const char **argv);
   "%:cpu_to_as(%{mcpu=*:%*}) %{mspfp*} %{mdpfp*} "                      \
   "%{mfpu=fpuda*:-mfpuda} %{mcode-density}"
 
+/* Support for a compile-time default CPU and FPU.  The rules are:
+   --with-cpu is ignored if -mcpu, mARC*, marc*, mA7, mA6 are specified.
+   --with-fpu is ignored if -mfpu is specified.  */
 #define OPTION_DEFAULT_SPECS						\
+  {"fpu", "%{!mfpu=*:-mfpu=%(VALUE)}"},					\
   {"cpu", "%{!mcpu=*:%{!mARC*:%{!marc*:%{!mA7:%{!mA6:-mcpu=%(VALUE)}}}}}" }
 
 #ifndef DRIVER_ENDIAN_SELF_SPECS
-- 
2.31.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arc: Add --with-fpu support for ARCv2 cpus
  2021-06-09 11:35       ` Claudiu Zissulescu
@ 2021-06-09 12:26         ` Bernhard Reutner-Fischer
  2021-06-09 14:48           ` Jeff Law
  2021-06-11 11:25           ` Claudiu Zissulescu
  0 siblings, 2 replies; 12+ messages in thread
From: Bernhard Reutner-Fischer @ 2021-06-09 12:26 UTC (permalink / raw)
  To: Claudiu Zissulescu; +Cc: rep.dot.nop, Jeff Law, gcc-patches, fbedard

On Wed, 9 Jun 2021 14:35:01 +0300
Claudiu Zissulescu <claziss@gmail.com> wrote:

> > ISTM you only set the expected flags in the switch so i would have
> > set only that variable and have grepped only once after the switch for
> > brevity.  
> 
> ARC has various FPU extensions, some of them are common to EM and HS 
> architectures, others are specific for only one of them. Hence, the grep 
> commands are ensuring that we accept the right fpu extension for the 
> right ARC architecture.

Right. Which you'd accomplish more terse if you would set flags_ok in
the switch cited below and after the switch have one single grep à la

if [ -n "$flags_ok" ] \
  && ! grep -q -E "^ARC_CPU[[:blank:]]*\($new_cpu,[[:blank:]]*$flags_ok," \
         ${srcdir}/config/arc/arc-cpus.def
then
  echo "Unknown floating point type used in "\
       "--with-fpu=$with_fpu for cpu $new_cpu" 1>&2
  exit 1
fi

The reason is that all case statements in the $with_fpu switch just
differ in the allowed flags AFAICS. But as you prefer.

last comments below.

> diff --git a/gcc/config.gcc b/gcc/config.gcc
> index 13c2004e3c52..09886c8635e0 100644
> --- a/gcc/config.gcc
> +++ b/gcc/config.gcc
> @@ -4258,18 +4258,61 @@ case "${target}" in
>  		;;
>  
>  	arc*-*-*)
> -		supported_defaults="cpu"
> +		supported_defaults="cpu fpu"
>  
> +		new_cpu=hs38_linux
>  		if [ x"$with_cpu" = x ] \
> -		    || grep "^ARC_CPU ($with_cpu," \
> +		    || grep -E "^ARC_CPU \($with_cpu," \
>  		       ${srcdir}/config/arc/arc-cpus.def \
>  		       > /dev/null; then

Cosmetics: You may want to keep the non "-E" version but use -q
and drop the redirect to /dev/null.

>  		 # Ok
> -		 true
> +		 new_cpu=$with_cpu
>  		else
>  		 echo "Unknown cpu used in --with-cpu=$with_cpu" 1>&2
>  		 exit 1
>  		fi
> +
> +		# see if --with-fpu matches any of the supported FPUs
> +		case "$with_fpu" in
> +		"")
> +			# OK
> +			;;
> +		fpus | fpus_div | fpus_fma | fpus_all)
> +			# OK if em or hs
> +			if ! grep -q -E "^ARC_CPU[[:blank:]]*\($new_cpu,[[:space:]]*[emhs]+," \

you changed only the first :space: to :blank: (everywhere)

> +			   ${srcdir}/config/arc/arc-cpus.def
> +			then
> +			 echo "Unknown floating point type used in "\
> +			 "--with-fpu=$with_fpu for cpu $new_cpu" 1>&2
> +			 exit 1
> +			fi
> +		;;
> +		fpuda | fpuda_div | fpuda_fma | fpuda_all)
> +			# OK only em
> +			if ! grep -q -E "^ARC_CPU[[:blank:]]*\($new_cpu,[[:space:]]*em," \
> +			   ${srcdir}/config/arc/arc-cpus.def
> +			then
> +			 echo "Unknown floating point type used in "\
> +			      "--with-fpu=$with_fpu for cpu $new_cpu" 1>&2
> +			 exit 1
> +			fi
> +			;;
> +		fpud | fpud_div | fpud_fma | fpud_all)
> +			# OK only hs
> +			if ! grep -q -E "^ARC_CPU[[:blank:]]*\($new_cpu,[[:space:]]*hs," \
> +			   ${srcdir}/config/arc/arc-cpus.def
> +			then
> +			 echo "Unknown floating point type used in"\
> +			      "--with-fpu=$with_fpu for cpu $new_cpu" 1>&2

missing trailing space after 'in' in "used in"\

LGTM with that fixed with or without cutting down on grep lines,
but of course i cannot approve it.
thanks,

> +			 exit 1
> +			fi
> +			;;
> +		*)
> +			echo "Unknown floating point type used in "\
> +			     "--with-fpu=$with_fpu" 1>&2
> +			exit 1
> +			;;
> +		esac
>  		;;
>  
>      csky-*-*)


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arc: Add --with-fpu support for ARCv2 cpus
  2021-06-09 12:26         ` Bernhard Reutner-Fischer
@ 2021-06-09 14:48           ` Jeff Law
  2021-06-11 11:25           ` Claudiu Zissulescu
  1 sibling, 0 replies; 12+ messages in thread
From: Jeff Law @ 2021-06-09 14:48 UTC (permalink / raw)
  To: Bernhard Reutner-Fischer, Claudiu Zissulescu; +Cc: gcc-patches, fbedard



On 6/9/2021 6:26 AM, Bernhard Reutner-Fischer wrote:
> On Wed, 9 Jun 2021 14:35:01 +0300
> Claudiu Zissulescu <claziss@gmail.com> wrote:
>
>>> ISTM you only set the expected flags in the switch so i would have
>>> set only that variable and have grepped only once after the switch for
>>> brevity.
>> ARC has various FPU extensions, some of them are common to EM and HS
>> architectures, others are specific for only one of them. Hence, the grep
>> commands are ensuring that we accept the right fpu extension for the
>> right ARC architecture.
> Right. Which you'd accomplish more terse if you would set flags_ok in
> the switch cited below and after the switch have one single grep à la
>
> if [ -n "$flags_ok" ] \
>    && ! grep -q -E "^ARC_CPU[[:blank:]]*\($new_cpu,[[:blank:]]*$flags_ok," \
>           ${srcdir}/config/arc/arc-cpus.def
> then
>    echo "Unknown floating point type used in "\
>         "--with-fpu=$with_fpu for cpu $new_cpu" 1>&2
>    exit 1
> fi
>
> The reason is that all case statements in the $with_fpu switch just
> differ in the allowed flags AFAICS. But as you prefer.
>
> last comments below.
>
>> diff --git a/gcc/config.gcc b/gcc/config.gcc
>> index 13c2004e3c52..09886c8635e0 100644
>> --- a/gcc/config.gcc
>> +++ b/gcc/config.gcc
>> @@ -4258,18 +4258,61 @@ case "${target}" in
>>   		;;
>>   
>>   	arc*-*-*)
>> -		supported_defaults="cpu"
>> +		supported_defaults="cpu fpu"
>>   
>> +		new_cpu=hs38_linux
>>   		if [ x"$with_cpu" = x ] \
>> -		    || grep "^ARC_CPU ($with_cpu," \
>> +		    || grep -E "^ARC_CPU \($with_cpu," \
>>   		       ${srcdir}/config/arc/arc-cpus.def \
>>   		       > /dev/null; then
> Cosmetics: You may want to keep the non "-E" version but use -q
> and drop the redirect to /dev/null.
>
>>   		 # Ok
>> -		 true
>> +		 new_cpu=$with_cpu
>>   		else
>>   		 echo "Unknown cpu used in --with-cpu=$with_cpu" 1>&2
>>   		 exit 1
>>   		fi
>> +
>> +		# see if --with-fpu matches any of the supported FPUs
>> +		case "$with_fpu" in
>> +		"")
>> +			# OK
>> +			;;
>> +		fpus | fpus_div | fpus_fma | fpus_all)
>> +			# OK if em or hs
>> +			if ! grep -q -E "^ARC_CPU[[:blank:]]*\($new_cpu,[[:space:]]*[emhs]+," \
> you changed only the first :space: to :blank: (everywhere)
>
>> +			   ${srcdir}/config/arc/arc-cpus.def
>> +			then
>> +			 echo "Unknown floating point type used in "\
>> +			 "--with-fpu=$with_fpu for cpu $new_cpu" 1>&2
>> +			 exit 1
>> +			fi
>> +		;;
>> +		fpuda | fpuda_div | fpuda_fma | fpuda_all)
>> +			# OK only em
>> +			if ! grep -q -E "^ARC_CPU[[:blank:]]*\($new_cpu,[[:space:]]*em," \
>> +			   ${srcdir}/config/arc/arc-cpus.def
>> +			then
>> +			 echo "Unknown floating point type used in "\
>> +			      "--with-fpu=$with_fpu for cpu $new_cpu" 1>&2
>> +			 exit 1
>> +			fi
>> +			;;
>> +		fpud | fpud_div | fpud_fma | fpud_all)
>> +			# OK only hs
>> +			if ! grep -q -E "^ARC_CPU[[:blank:]]*\($new_cpu,[[:space:]]*hs," \
>> +			   ${srcdir}/config/arc/arc-cpus.def
>> +			then
>> +			 echo "Unknown floating point type used in"\
>> +			      "--with-fpu=$with_fpu for cpu $new_cpu" 1>&2
> missing trailing space after 'in' in "used in"\
>
> LGTM with that fixed with or without cutting down on grep lines,
> but of course i cannot approve it.
But your comments are greatly appreciated ;-)

ACK'd for the trunk once the issues Bernhard mentioned are addressed.

jeff


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arc: Add --with-fpu support for ARCv2 cpus
  2021-06-09 12:26         ` Bernhard Reutner-Fischer
  2021-06-09 14:48           ` Jeff Law
@ 2021-06-11 11:25           ` Claudiu Zissulescu
  2021-06-13 10:06             ` Bernhard Reutner-Fischer
  1 sibling, 1 reply; 12+ messages in thread
From: Claudiu Zissulescu @ 2021-06-11 11:25 UTC (permalink / raw)
  To: Bernhard Reutner-Fischer; +Cc: Jeff Law, gcc-patches, fbedard

[-- Attachment #1: Type: text/plain, Size: 129 bytes --]

Hi Bernhard,

Please find attached my latest patch, it includes (hopefully) all your 
feedback.

Thank you for comments,
Claudiu

[-- Attachment #2: 0001-arc-Add-with-fpu-support-for-ARCv2-cpus.patch --]
[-- Type: text/x-patch, Size: 2981 bytes --]

From 03075b3d9194120d7adb3cdc2aa0f58e3ea9dd1d Mon Sep 17 00:00:00 2001
From: Claudiu Zissulescu <claziss@synopsys.com>
Date: Wed, 21 Oct 2020 16:11:43 +0300
Subject: [PATCH] arc: Add --with-fpu support for ARCv2 cpus

Support for a compile-time default FPU. The --with-fpu configuration
option is ignored if -mfpu compiler option is specified. The FPU
options are only available for ARCv2 cpus.

gcc/
yyyy-mm-dd  Claudiu Zissulescu  <claziss@synopsys.com>

	* config.gcc (arc): Add support for with_cpu option.
	* config/arc/arc.h (OPTION_DEFAULT_SPECS): Add fpu.

Signed-off-by: Claudiu Zissulescu <claziss@synopsys.com>
---
 gcc/config.gcc       | 44 +++++++++++++++++++++++++++++++++++++++-----
 gcc/config/arc/arc.h |  4 ++++
 2 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/gcc/config.gcc b/gcc/config.gcc
index 13c2004e3c52..5581dae88b52 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -4258,18 +4258,52 @@ case "${target}" in
 		;;
 
 	arc*-*-*)
-		supported_defaults="cpu"
+		supported_defaults="cpu fpu"
 
+		new_cpu=hs38_linux
 		if [ x"$with_cpu" = x ] \
-		    || grep "^ARC_CPU ($with_cpu," \
-		       ${srcdir}/config/arc/arc-cpus.def \
-		       > /dev/null; then
+		    || grep -q -E "^ARC_CPU[[:blank:]]*\($with_cpu," \
+		       ${srcdir}/config/arc/arc-cpus.def
+		then
 		 # Ok
-		 true
+		 new_cpu=$with_cpu
 		else
 		 echo "Unknown cpu used in --with-cpu=$with_cpu" 1>&2
 		 exit 1
 		fi
+
+		# see if --with-fpu matches any of the supported FPUs
+		case "$with_fpu" in
+		"")
+			# OK
+			;;
+		fpus | fpus_div | fpus_fma | fpus_all)
+			# OK if em or hs
+			flags_ok="[emhs]+"
+			;;
+		fpuda | fpuda_div | fpuda_fma | fpuda_all)
+			# OK only em
+			flags_ok="em"
+			;;
+		fpud | fpud_div | fpud_fma | fpud_all)
+			# OK only hs
+			flags_ok="hs"
+			;;
+		*)
+			echo "Unknown floating point type used in "\
+			     "--with-fpu=$with_fpu" 1>&2
+			exit 1
+			;;
+		esac
+
+		if [ -n "$flags_ok" ] \
+		   && ! grep -q -E "^ARC_CPU[[:blank:]]*\($new_cpu,[[:blank:]]*$flags_ok," \
+		   ${srcdir}/config/arc/arc-cpus.def
+		then
+		   echo "Unknown floating point type used in "\
+			 "--with-fpu=$with_fpu for cpu $new_cpu" 1>&2
+			 exit 1
+		fi
 		;;
 
     csky-*-*)
diff --git a/gcc/config/arc/arc.h b/gcc/config/arc/arc.h
index 2ccebfa7afe6..e78f6e8202b3 100644
--- a/gcc/config/arc/arc.h
+++ b/gcc/config/arc/arc.h
@@ -100,7 +100,11 @@ extern const char *arc_cpu_to_as (int argc, const char **argv);
   "%:cpu_to_as(%{mcpu=*:%*}) %{mspfp*} %{mdpfp*} "                      \
   "%{mfpu=fpuda*:-mfpuda} %{mcode-density}"
 
+/* Support for a compile-time default CPU and FPU.  The rules are:
+   --with-cpu is ignored if -mcpu, mARC*, marc*, mA7, mA6 are specified.
+   --with-fpu is ignored if -mfpu is specified.  */
 #define OPTION_DEFAULT_SPECS						\
+  {"fpu", "%{!mfpu=*:-mfpu=%(VALUE)}"},					\
   {"cpu", "%{!mcpu=*:%{!mARC*:%{!marc*:%{!mA7:%{!mA6:-mcpu=%(VALUE)}}}}}" }
 
 #ifndef DRIVER_ENDIAN_SELF_SPECS
-- 
2.31.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arc: Add --with-fpu support for ARCv2 cpus
  2021-06-11 11:25           ` Claudiu Zissulescu
@ 2021-06-13 10:06             ` Bernhard Reutner-Fischer
  2021-06-13 21:34               ` Jeff Law
  0 siblings, 1 reply; 12+ messages in thread
From: Bernhard Reutner-Fischer @ 2021-06-13 10:06 UTC (permalink / raw)
  To: Claudiu Zissulescu; +Cc: rep.dot.nop, Jeff Law, gcc-patches, fbedard

On Fri, 11 Jun 2021 14:25:24 +0300
Claudiu Zissulescu <claziss@gmail.com> wrote:

> Hi Bernhard,
> 
> Please find attached my latest patch, it includes (hopefully) all your 
> feedback.
> 
> Thank you for comments,

concise and clean, i wouldn't know what to remove. LGTM.
thanks for your patience!

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arc: Add --with-fpu support for ARCv2 cpus
  2021-06-13 10:06             ` Bernhard Reutner-Fischer
@ 2021-06-13 21:34               ` Jeff Law
  2021-06-14 12:35                 ` Claudiu Zissulescu Ianculescu
  0 siblings, 1 reply; 12+ messages in thread
From: Jeff Law @ 2021-06-13 21:34 UTC (permalink / raw)
  To: Bernhard Reutner-Fischer, Claudiu Zissulescu; +Cc: gcc-patches, fbedard



On 6/13/2021 4:06 AM, Bernhard Reutner-Fischer wrote:
> On Fri, 11 Jun 2021 14:25:24 +0300
> Claudiu Zissulescu <claziss@gmail.com> wrote:
>
>> Hi Bernhard,
>>
>> Please find attached my latest patch, it includes (hopefully) all your
>> feedback.
>>
>> Thank you for comments,
> concise and clean, i wouldn't know what to remove. LGTM.
> thanks for your patience!
THen let's consider it approved at this point.  Thanks for chiming in 
Bernhard and thanks for implementing the suggestions Claudiu!

jeff

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arc: Add --with-fpu support for ARCv2 cpus
  2021-06-13 21:34               ` Jeff Law
@ 2021-06-14 12:35                 ` Claudiu Zissulescu Ianculescu
  0 siblings, 0 replies; 12+ messages in thread
From: Claudiu Zissulescu Ianculescu @ 2021-06-14 12:35 UTC (permalink / raw)
  To: Jeff Law; +Cc: Bernhard Reutner-Fischer, gcc-patches, Francois Bedard

Thanks a lot guys. Patch is pushed.

//Claudiu

On Mon, Jun 14, 2021 at 12:34 AM Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
>
> On 6/13/2021 4:06 AM, Bernhard Reutner-Fischer wrote:
> > On Fri, 11 Jun 2021 14:25:24 +0300
> > Claudiu Zissulescu <claziss@gmail.com> wrote:
> >
> >> Hi Bernhard,
> >>
> >> Please find attached my latest patch, it includes (hopefully) all your
> >> feedback.
> >>
> >> Thank you for comments,
> > concise and clean, i wouldn't know what to remove. LGTM.
> > thanks for your patience!
> THen let's consider it approved at this point.  Thanks for chiming in
> Bernhard and thanks for implementing the suggestions Claudiu!
>
> jeff

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-06-14 12:35 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-04  7:29 [PATCH] arc: Add --with-fpu support for ARCv2 cpus Claudiu Zissulescu
2021-06-04  9:04 ` Bernhard Reutner-Fischer
2021-06-05  5:46 ` Jeff Law
2021-06-08  7:05   ` Claudiu Zissulescu
2021-06-08 10:19     ` Bernhard Reutner-Fischer
2021-06-09 11:35       ` Claudiu Zissulescu
2021-06-09 12:26         ` Bernhard Reutner-Fischer
2021-06-09 14:48           ` Jeff Law
2021-06-11 11:25           ` Claudiu Zissulescu
2021-06-13 10:06             ` Bernhard Reutner-Fischer
2021-06-13 21:34               ` Jeff Law
2021-06-14 12:35                 ` Claudiu Zissulescu Ianculescu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).