public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [AArch64] Add --with-tune configure flag
@ 2020-11-17 17:53 Pop, Sebastian
  2020-11-17 21:27 ` Pop, Sebastian
  2020-11-17 21:43 ` Jeff Law
  0 siblings, 2 replies; 15+ messages in thread
From: Pop, Sebastian @ 2020-11-17 17:53 UTC (permalink / raw)
  To: gcc-patches

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

Hi,

the attached patch fixes a configure error on Arm64 when passing --with-tune=... to configure:
```
This target does not support --with-tune.
Valid --with options are: abi cpu arch
```
The missing flag sets target tuning to a different value than the generic tuning.

gcc/
                * config.gcc: Add --with-tune to AArch64 configure flags.

Tested on aarch64-linux with bootstrap and regression test.
Ok to commit to trunk and back-port to active branches?

Thanks,
Sebastian


[-- Attachment #2: 0001-AArch64-add-with-tune-configure-flag.patch --]
[-- Type: application/octet-stream, Size: 1084 bytes --]

From f4cd05212a9d290648515b93157699eb1400dff9 Mon Sep 17 00:00:00 2001
From: Sebastian Pop <spop@amazon.com>
Date: Tue, 17 Nov 2020 16:00:51 +0000
Subject: [PATCH] [AArch64] add --with-tune configure flag

fixes a configure error on Arm64 when passing --with-tune=... to configure:
```
This target does not support --with-tune.
Valid --with options are: abi cpu arch
```
The missing flag sets target tuning to a different value than generic tuning.

gcc/
	* config.gcc: Add --with-tune to AArch64 configure flags.
---
 gcc/config.gcc | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/gcc/config.gcc b/gcc/config.gcc
index b42ebc4e5be..25ef648c288 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -4065,9 +4065,8 @@ fi
 supported_defaults=
 case "${target}" in
 	aarch64*-*-*)
-		supported_defaults="abi cpu arch"
-		for which in cpu arch; do
-
+		supported_defaults="abi cpu arch tune"
+		for which in cpu arch tune; do
 			eval "val=\$with_$which"
 			base_val=`echo $val | sed -e 's/\+.*//'`
 			ext_val=`echo $val | sed -e 's/[a-z0-9.-]\+//'`
-- 
2.25.1


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

* Re: [AArch64] Add --with-tune configure flag
  2020-11-17 17:53 [AArch64] Add --with-tune configure flag Pop, Sebastian
@ 2020-11-17 21:27 ` Pop, Sebastian
  2020-11-17 21:44   ` Jeff Law
  2020-11-17 21:43 ` Jeff Law
  1 sibling, 1 reply; 15+ messages in thread
From: Pop, Sebastian @ 2020-11-17 21:27 UTC (permalink / raw)
  To: gcc-patches

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

Hi,

here is a follow-up patch to add missing Arm64 configure flags as aliases to the existing flags.

gcc/
        * config.gcc: add configure flags --with-{cpu,arch,tune}-{32,64}
        as alias flags for --with-{cpu,arch,tune} on AArch64.
        * doc/install.texi: Document new flags for aarch64.

Tested on aarch64-linux with bootstrap and regression test.
Ok to commit to trunk and back-port to active branches?

Thanks,
Sebastian


[-- Attachment #2: 0001-AArch64-add-with-cpu-arch-tune-32-64-as-alias-flags-.patch --]
[-- Type: application/octet-stream, Size: 2273 bytes --]

From 54e91e950e8758eb089d451341cc3b5a524b4483 Mon Sep 17 00:00:00 2001
From: Sebastian Pop <spop@amazon.com>
Date: Tue, 17 Nov 2020 18:56:15 +0000
Subject: [PATCH] [AArch64] add --with-{cpu,arch,tune}-{32,64} as alias flags
 for --with-{cpu,arch,tune}

gcc/
	* config.gcc: add configure flags --with-{cpu,arch,tune}-{32,64}
	as alias flags for --with-{cpu,arch,tune} on AArch64.
	* doc/install.texi: Document new flags for aarch64.
---
 gcc/config.gcc       | 20 +++++++++++++++++++-
 gcc/doc/install.texi |  2 +-
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/gcc/config.gcc b/gcc/config.gcc
index 25ef648c288..380785b3f7b 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -4065,7 +4065,25 @@ fi
 supported_defaults=
 case "${target}" in
 	aarch64*-*-*)
-		supported_defaults="abi cpu arch tune"
+		supported_defaults="abi cpu cpu_32 cpu_64 arch arch_32 arch_64 tune tune_32 tune_64"
+		if test x$with_cpu_64 != x && test x$with_cpu = x; then
+			with_cpu=$with_cpu_64
+		fi
+		if test x$with_cpu_32 != x && test x$with_cpu = x; then
+			with_cpu=$with_cpu_32
+		fi
+		if test x$with_arch_64 != x && test x$with_arch = x; then
+			with_arch=$with_arch_64
+		fi
+		if test x$with_arch_32 != x && test x$with_arch = x; then
+			with_arch=$with_arch_32
+		fi
+		if test x$with_tune_64 != x && test x$with_tune = x; then
+			with_tune=$with_tune_64
+		fi
+		if test x$with_tune_32 != x && test x$with_tune = x; then
+			with_tune=$with_tune_32
+		fi
 		for which in cpu arch tune; do
 			eval "val=\$with_$which"
 			base_val=`echo $val | sed -e 's/\+.*//'`
diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
index 876b04f9c45..b7757b544a4 100644
--- a/gcc/doc/install.texi
+++ b/gcc/doc/install.texi
@@ -1338,7 +1338,7 @@ Specify which cpu variant the compiler should generate code for by default.
 This option is only supported on some targets, including ARC, ARM, i386, M68k,
 PowerPC, and SPARC@.  It is mandatory for ARC@.  The @option{--with-cpu-32} and
 @option{--with-cpu-64} options specify separate default CPUs for
-32-bit and 64-bit modes; these options are only supported for i386,
+32-bit and 64-bit modes; these options are only supported for aarch64, i386,
 x86-64, PowerPC, and SPARC@.
 
 @item --with-schedule=@var{cpu}
-- 
2.25.1


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

* Re: [AArch64] Add --with-tune configure flag
  2020-11-17 17:53 [AArch64] Add --with-tune configure flag Pop, Sebastian
  2020-11-17 21:27 ` Pop, Sebastian
@ 2020-11-17 21:43 ` Jeff Law
  1 sibling, 0 replies; 15+ messages in thread
From: Jeff Law @ 2020-11-17 21:43 UTC (permalink / raw)
  To: Pop, Sebastian, gcc-patches



On 11/17/20 10:53 AM, Pop, Sebastian via Gcc-patches wrote:
> Hi,
>
> the attached patch fixes a configure error on Arm64 when passing --with-tune=... to configure:
> ```
> This target does not support --with-tune.
> Valid --with options are: abi cpu arch
> ```
> The missing flag sets target tuning to a different value than the generic tuning.
>
> gcc/
>                 * config.gcc: Add --with-tune to AArch64 configure flags.
>
> Tested on aarch64-linux with bootstrap and regression test.
> Ok to commit to trunk and back-port to active branches?
OK

jeff


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

* Re: [AArch64] Add --with-tune configure flag
  2020-11-17 21:27 ` Pop, Sebastian
@ 2020-11-17 21:44   ` Jeff Law
  0 siblings, 0 replies; 15+ messages in thread
From: Jeff Law @ 2020-11-17 21:44 UTC (permalink / raw)
  To: Pop, Sebastian, gcc-patches



On 11/17/20 2:27 PM, Pop, Sebastian via Gcc-patches wrote:
> Hi,
>
> here is a follow-up patch to add missing Arm64 configure flags as aliases to the existing flags.
>
> gcc/
>         * config.gcc: add configure flags --with-{cpu,arch,tune}-{32,64}
>         as alias flags for --with-{cpu,arch,tune} on AArch64.
>         * doc/install.texi: Document new flags for aarch64.
>
> Tested on aarch64-linux with bootstrap and regression test.
> Ok to commit to trunk and back-port to active branches?
OK
jeff


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

* Re: [AArch64] Add --with-tune configure flag
  2020-12-07 20:32     ` Richard Sandiford
  2020-12-07 22:45       ` Wilco Dijkstra
@ 2020-12-10 19:02       ` Wilco Dijkstra
  1 sibling, 0 replies; 15+ messages in thread
From: Wilco Dijkstra @ 2020-12-10 19:02 UTC (permalink / raw)
  To: Richard Sandiford, Wilco Dijkstra via Gcc-patches
  Cc: Pop, Sebastian, Richard Earnshaw

Hi Richard,

> I specifically want to test generic SVE rather than SVE tuned for a
> specific core, so --with-arch=armv8.2-a+sve is the thing I want to test.

Btw that's not actually what you get if you use cc1 - you always get armv8.0,
so --with-arch doesn't work at all. The only case that appears to work in cc1
is --with-cpu as long as it is not --with-cpu=native.

Cheers,
Wilco

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

* Re: [AArch64] Add --with-tune configure flag
  2020-12-07 20:32     ` Richard Sandiford
@ 2020-12-07 22:45       ` Wilco Dijkstra
  2020-12-10 19:02       ` Wilco Dijkstra
  1 sibling, 0 replies; 15+ messages in thread
From: Wilco Dijkstra @ 2020-12-07 22:45 UTC (permalink / raw)
  To: Richard Sandiford, Wilco Dijkstra via Gcc-patches
  Cc: Pop, Sebastian, Richard Earnshaw

Hi Richard,

>>> I share Richard E's concern about the effect of this on people who run
>>> ./cc1 directly.  (And I'm being selfish here, because I regularly run
>>> ./cc1 directly on toolchains configured with --with-arch=armv8.2-a+sve.)
>>> So TBH my preference would be to keep the TARGET_CPU_DEFAULT_FLAGS stuff.
>>
>> And what exactly should it do? It didn't work correctly before, and even now it's not
>> clear what TARGET_CPU_DEFAULT should be set to for all possible combinations of
>> --with-cpu=X --with-arch=Y --with-tune=Z. Should it have exactly the same behaviour
>> with and without the driver? If so, that's difficult to achieve in config.gcc, and it would
>> require a completely different mechanism to ensure the default ends up the same
>> without the driver (eg. store the original strings in TARGET_CPU_DEFAULT and then
>> postprocess in the backend as if they were actual --cpu/--arch/--tune commands if
>> the command-line didn't already set them, ie. replicating what the driver already does).
>> And it would need to be done for --with-abi as well.
>
> The specific case I mentioned works well enough though (given that I also
> run ./cc1 without -march, -mcpu or -mtune flags).  I agree it might not
> work for all combinations of --with-arch, --with-cpu and --with-tune,
> but I'm not sure that's a strong reason to remove the cases that do work
> (or at least, work well enough).  This cc1 behaviour is a feature for
> developers rather than users after all.

Well how do you suggest we keep the cases that work but block the cases that
don't work, but only in cc1 without knowing when someone uses cc1 without the
driver?

> Perhaps I've missed the point, but why do you need to remove this code
> in order to do what you need to do?

Basically it's buggy by design. We have 3 related options which are compressed into
a single CPU value plus a set of architecture extension flags. That obviously means
we can't describe the --with- flags, let alone any combinations, right?

For example the architecture to compile for is taken from that single CPU, so the
architecture is often wrong (depending on the order of processing in config.gcc).
Try bootstrapping trunk using --with-tune=neoverse-n2 to see what I mean.
It doesn't just set the tune, it forces an incorrect architecture!

My patch works precisely because I ignore the incorrect TARGET_CPU_DEFAULT
and allow the driver to do its job properly in prioritizing the options. Then it's
a simple matter of reading out the --cpu/arch/tune flags.

Like I said, it may be possible to fix this if we pass the --with strings to the backend
and process them there. But that means duplicating functionality of the driver for
a few users who use cc1. Is that really worth the effort?

>> So is changing your preferred config to --with-cpu=neoverse-v1 really a problem?
>
> I specifically want to test generic SVE rather than SVE tuned for a
> specific core, so --with-arch=armv8.2-a+sve is the thing I want to test.
> I think the question is instead whether it's really a problem to change
> my workflow so that I don't run cc1 directly any more, or that I remember
> to respecify the --with-arch value as an -march flag when running cc1.
> And yeah, maybe the right answer is that I should learn to do that.
> It's just that I don't personally understand why this change is necessary.

How about creating a macro that adds the --arch for you? Eg. cc1 <args>
becomes ./cc1 -march=armv8.2-a+sve <args>

Cheers,
Wilco

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

* Re: [AArch64] Add --with-tune configure flag
  2020-12-07 18:52   ` Wilco Dijkstra
@ 2020-12-07 20:32     ` Richard Sandiford
  2020-12-07 22:45       ` Wilco Dijkstra
  2020-12-10 19:02       ` Wilco Dijkstra
  0 siblings, 2 replies; 15+ messages in thread
From: Richard Sandiford @ 2020-12-07 20:32 UTC (permalink / raw)
  To: Wilco Dijkstra via Gcc-patches
  Cc: Pop, Sebastian, Wilco Dijkstra, Richard Earnshaw

Wilco Dijkstra via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> Hi Richard,
>
>> I share Richard E's concern about the effect of this on people who run
>> ./cc1 directly.  (And I'm being selfish here, because I regularly run
>> ./cc1 directly on toolchains configured with --with-arch=armv8.2-a+sve.)
>> So TBH my preference would be to keep the TARGET_CPU_DEFAULT_FLAGS stuff.
>
> And what exactly should it do? It didn't work correctly before, and even now it's not
> clear what TARGET_CPU_DEFAULT should be set to for all possible combinations of
> --with-cpu=X --with-arch=Y --with-tune=Z. Should it have exactly the same behaviour
> with and without the driver? If so, that's difficult to achieve in config.gcc, and it would
> require a completely different mechanism to ensure the default ends up the same
> without the driver (eg. store the original strings in TARGET_CPU_DEFAULT and then
> postprocess in the backend as if they were actual --cpu/--arch/--tune commands if
> the command-line didn't already set them, ie. replicating what the driver already does).
> And it would need to be done for --with-abi as well.

The specific case I mentioned works well enough though (given that I also
run ./cc1 without -march, -mcpu or -mtune flags).  I agree it might not
work for all combinations of --with-arch, --with-cpu and --with-tune,
but I'm not sure that's a strong reason to remove the cases that do work
(or at least, work well enough).  This cc1 behaviour is a feature for
developers rather than users after all.

Perhaps I've missed the point, but why do you need to remove this code
in order to do what you need to do?

> So is changing your preferred config to --with-cpu=neoverse-v1 really a problem?

I specifically want to test generic SVE rather than SVE tuned for a
specific core, so --with-arch=armv8.2-a+sve is the thing I want to test.
I think the question is instead whether it's really a problem to change
my workflow so that I don't run cc1 directly any more, or that I remember
to respecify the --with-arch value as an -march flag when running cc1.
And yeah, maybe the right answer is that I should learn to do that.
It's just that I don't personally understand why this change is necessary.

Like I say though, I'm happy to defer to a maintainer who does think the
patch is a good thing.

Thanks,
Richard

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

* Re: [AArch64] Add --with-tune configure flag
  2020-12-07 17:33 ` Richard Sandiford
@ 2020-12-07 18:52   ` Wilco Dijkstra
  2020-12-07 20:32     ` Richard Sandiford
  0 siblings, 1 reply; 15+ messages in thread
From: Wilco Dijkstra @ 2020-12-07 18:52 UTC (permalink / raw)
  To: Richard Sandiford, Pop, Sebastian; +Cc: Richard Earnshaw, GCC Patches

Hi Richard,

> I share Richard E's concern about the effect of this on people who run
> ./cc1 directly.  (And I'm being selfish here, because I regularly run
> ./cc1 directly on toolchains configured with --with-arch=armv8.2-a+sve.)
> So TBH my preference would be to keep the TARGET_CPU_DEFAULT_FLAGS stuff.

And what exactly should it do? It didn't work correctly before, and even now it's not
clear what TARGET_CPU_DEFAULT should be set to for all possible combinations of
--with-cpu=X --with-arch=Y --with-tune=Z. Should it have exactly the same behaviour
with and without the driver? If so, that's difficult to achieve in config.gcc, and it would
require a completely different mechanism to ensure the default ends up the same
without the driver (eg. store the original strings in TARGET_CPU_DEFAULT and then
postprocess in the backend as if they were actual --cpu/--arch/--tune commands if
the command-line didn't already set them, ie. replicating what the driver already does).
And it would need to be done for --with-abi as well.

So is changing your preferred config to --with-cpu=neoverse-v1 really a problem?

Cheers,
Wilco

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

* Re: [AArch64] Add --with-tune configure flag
  2020-12-04 16:34 Pop, Sebastian
@ 2020-12-07 17:33 ` Richard Sandiford
  2020-12-07 18:52   ` Wilco Dijkstra
  0 siblings, 1 reply; 15+ messages in thread
From: Richard Sandiford @ 2020-12-07 17:33 UTC (permalink / raw)
  To: Pop, Sebastian; +Cc: Richard Earnshaw (lists), Wilco Dijkstra, GCC Patches

"Pop, Sebastian" <spop@amazon.com> writes:
> On 11/19/20, 10:52 AM, "Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com> wrote:
>> Having the same option have a completely different meaning would be even
>> worse than not having the option at all.  So no, that's a non-starter.
>
> The attached patch 0001 removes --with-{cpu,arch,tune}-32.
> Bootstrap and regression testing pass on aarch64-linux.
> Ok to commit to trunk and active branches?
>
> I would like to ping the two patches from Wilco Dijkstra that fix issues in configure --with-mtune flag:
> https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553865.html
> https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553866.html
>
> Please see patches 0002 and 0003 attached, rebased on trunk as of today and tested with bootstrap and regression testing.
> Ok to commit to trunk and all active branches?
>
> Thanks,
> Sebastian
>
> From f1aa5f45dde8df5eee41ae166176775f5c5f1acc Mon Sep 17 00:00:00 2001
> From: Sebastian Pop <spop@amazon.com>
> Date: Thu, 3 Dec 2020 17:35:18 +0000
> Subject: [PATCH 1/3] [AArch64] disable --with-{cpu,arch,tune}-32
>
> gcc/
> 	* config.gcc (aarch64*-*-*): Remove --with-{cpu,arch,tune}-32 flags.

OK, thanks.

> ChangeLog:
> 2020-09-03  Wilco Dijkstra  <wdijkstr@arm.com>
>
>         * config.gcc (aarch64*-*-*): Simplify --with-cpu and --with-arch
>         processing.  Add support for architectural extensions.
>         * config/aarch64/aarch64.h (TARGET_CPU_DEFAULT): Remove
>         AARCH64_CPU_DEFAULT_FLAGS.
>         * config/aarch64/aarch64.c (AARCH64_CPU_DEFAULT_FLAGS): Remove define.
>         (get_tune_cpu): Assert CPU is always valid.
>         (get_arch): Assert architecture is always valid.
>         (aarch64_override_options): Cleanup CPU selection code and simplify logic.

I share Richard E's concern about the effect of this on people who run
./cc1 directly.  (And I'm being selfish here, because I regularly run
./cc1 directly on toolchains configured with --with-arch=armv8.2-a+sve.)
So TBH my preference would be to keep the TARGET_CPU_DEFAULT_FLAGS stuff.

However, if one of the other maintainers strongly thinks this is the
right way to go, I'm happy to defer to them.

> Add support for --with-tune. Like --with-cpu and --with-arch, the argument is
> validated and transformed into a -mtune option to be processed like any other
> command-line option.  --with-tune has no effect if a -mcpu or -mtune option
> is used. The validating code didn't allow --with-cpu=native, so explicitly
> allow that.
>
> Co-authored-by:  Delia Burduv  <delia.burduv@arm.com>
>
> Bootstrap OK, regress pass, OK to commit?
>
> ChangeLog
> 2020-09-03  Wilco Dijkstra  <wdijkstr@arm.com>
>
>         * config.gcc
>         (aarch64*-*-*): Add --with-tune. Support --with-cpu=native.
>         * config/aarch64/aarch64.h (OPTION_DEFAULT_SPECS): Add --with-tune.
>
> 2020-09-03  Wilco Dijkstra  <wdijkstr@arm.com>
>
>         * gcc/testsuite/lib/target-supports.exp:
>         (check_effective_target_tune_cortex_a76): New effective target test.
>         * gcc.target/aarch64/with-tune-config.c: New test.
>         * gcc.target/aarch64/with-tune-march.c: Likewise.
>         * gcc.target/aarch64/with-tune-mcpu.c: Likewise.
>         * gcc.target/aarch64/with-tune-mtune.c: Likewise.

OK for this one, thanks.

Richard

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

* Re: [AArch64] Add --with-tune configure flag
@ 2020-12-04 16:34 Pop, Sebastian
  2020-12-07 17:33 ` Richard Sandiford
  0 siblings, 1 reply; 15+ messages in thread
From: Pop, Sebastian @ 2020-12-04 16:34 UTC (permalink / raw)
  To: Richard Earnshaw (lists), Wilco Dijkstra, GCC Patches; +Cc: Richard Sandiford

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

On 11/19/20, 10:52 AM, "Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com> wrote:
> Having the same option have a completely different meaning would be even
> worse than not having the option at all.  So no, that's a non-starter.

The attached patch 0001 removes --with-{cpu,arch,tune}-32.
Bootstrap and regression testing pass on aarch64-linux.
Ok to commit to trunk and active branches?

I would like to ping the two patches from Wilco Dijkstra that fix issues in configure --with-mtune flag:
https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553865.html
https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553866.html

Please see patches 0002 and 0003 attached, rebased on trunk as of today and tested with bootstrap and regression testing.
Ok to commit to trunk and all active branches?

Thanks,
Sebastian


[-- Attachment #2: 0001-AArch64-disable-with-cpu-arch-tune-32.patch --]
[-- Type: application/octet-stream, Size: 1419 bytes --]

From f1aa5f45dde8df5eee41ae166176775f5c5f1acc Mon Sep 17 00:00:00 2001
From: Sebastian Pop <spop@amazon.com>
Date: Thu, 3 Dec 2020 17:35:18 +0000
Subject: [PATCH 1/3] [AArch64] disable --with-{cpu,arch,tune}-32

gcc/
	* config.gcc (aarch64*-*-*): Remove --with-{cpu,arch,tune}-32 flags.
---
 gcc/config.gcc | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/gcc/config.gcc b/gcc/config.gcc
index 7b138d1bee1..86142b898d6 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -4091,25 +4091,16 @@ fi
 supported_defaults=
 case "${target}" in
 	aarch64*-*-*)
-		supported_defaults="abi cpu cpu_32 cpu_64 arch arch_32 arch_64 tune tune_32 tune_64"
+		supported_defaults="abi cpu cpu_64 arch arch_64 tune tune_64"
 		if test x$with_cpu_64 != x && test x$with_cpu = x; then
 			with_cpu=$with_cpu_64
 		fi
-		if test x$with_cpu_32 != x && test x$with_cpu = x; then
-			with_cpu=$with_cpu_32
-		fi
 		if test x$with_arch_64 != x && test x$with_arch = x; then
 			with_arch=$with_arch_64
 		fi
-		if test x$with_arch_32 != x && test x$with_arch = x; then
-			with_arch=$with_arch_32
-		fi
 		if test x$with_tune_64 != x && test x$with_tune = x; then
 			with_tune=$with_tune_64
 		fi
-		if test x$with_tune_32 != x && test x$with_tune = x; then
-			with_tune=$with_tune_32
-		fi
 		for which in cpu arch tune; do
 			eval "val=\$with_$which"
 			base_val=`echo $val | sed -e 's/\+.*//'`
-- 
2.25.1


[-- Attachment #3: 0002-AArch64-Cleanup-CPU-option-processing-code.patch --]
[-- Type: application/octet-stream, Size: 10924 bytes --]

From b734378fe706b9eb22dcc9a83a4aa8a23fcbcc28 Mon Sep 17 00:00:00 2001
From: Wilco Dijkstra <wdijkstr@arm.com>
Date: Thu, 3 Dec 2020 18:32:42 +0000
Subject: [PATCH 2/3] AArch64: Cleanup CPU option processing code

The --with-cpu/--with-arch configure option processing not only checks valid arguments
but also sets TARGET_CPU_DEFAULT with a CPU and extension bitmask.  This isn't used
however since a --with-cpu is translated into a -mcpu option which is processed as if
written on the command-line (so TARGET_CPU_DEFAULT is never accessed).

So remove all the complex processing and bitmask, and just validate the option.
Fix a bug that always reports valid architecture extensions as invalid.  As a result
the CPU processing in aarch64.c can be simplified.

Bootstrap OK, regress pass, OK for commit?

ChangeLog:
2020-09-03  Wilco Dijkstra  <wdijkstr@arm.com>

        * config.gcc (aarch64*-*-*): Simplify --with-cpu and --with-arch
        processing.  Add support for architectural extensions.
        * config/aarch64/aarch64.h (TARGET_CPU_DEFAULT): Remove
        AARCH64_CPU_DEFAULT_FLAGS.
        * config/aarch64/aarch64.c (AARCH64_CPU_DEFAULT_FLAGS): Remove define.
        (get_tune_cpu): Assert CPU is always valid.
        (get_arch): Assert architecture is always valid.
        (aarch64_override_options): Cleanup CPU selection code and simplify logic.
---
 gcc/config.gcc               |  43 +-------------
 gcc/config/aarch64/aarch64.c | 105 ++++++++++-------------------------
 gcc/config/aarch64/aarch64.h |   3 +-
 3 files changed, 31 insertions(+), 120 deletions(-)

diff --git a/gcc/config.gcc b/gcc/config.gcc
index 86142b898d6..7cdfc95ecee 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -4114,8 +4114,6 @@ case "${target}" in
 			  pattern=AARCH64_CORE
 			fi
 
-			ext_mask=AARCH64_CPU_DEFAULT_FLAGS
-
 			# Find the base CPU or ARCH id in aarch64-cores.def or
 			# aarch64-arches.def
 			if [ x"$base_val" = x ] \
@@ -4123,23 +4121,6 @@ case "${target}" in
 				    ${srcdir}/config/aarch64/$def \
 				    > /dev/null; then
 
-			  if [ $which = arch ]; then
-				base_id=`grep "^$pattern(\"$base_val\"," \
-				  ${srcdir}/config/aarch64/$def | \
-				  sed -e 's/^[^,]*,[ 	]*//' | \
-				  sed -e 's/,.*$//'`
-				# Extract the architecture flags from aarch64-arches.def
-				ext_mask=`grep "^$pattern(\"$base_val\"," \
-				   ${srcdir}/config/aarch64/$def | \
-				   sed -e 's/)$//' | \
-				   sed -e 's/^.*,//'`
-			  else
-				base_id=`grep "^$pattern(\"$base_val\"," \
-				  ${srcdir}/config/aarch64/$def | \
-				  sed -e 's/^[^,]*,[ 	]*//' | \
-				  sed -e 's/,.*$//'`
-			  fi
-
 			  # Use the pre-processor to strip flatten the options.
 			  # This makes the format less rigid than if we use
 			  # grep and sed directly here.
@@ -4164,25 +4145,7 @@ case "${target}" in
 					grep "^\"$base_ext\""`
 
 				if [ x"$base_ext" = x ] \
-				    || [[ -n $opt_line ]]; then
-
-				  # These regexp extract the elements based on
-				  # their group match index in the regexp.
-				  ext_canon=`echo -e "$opt_line" | \
-					sed -e "s/$sed_patt/\2/"`
-				  ext_on=`echo -e "$opt_line" | \
-					sed -e "s/$sed_patt/\3/"`
-				  ext_off=`echo -e "$opt_line" | \
-					sed -e "s/$sed_patt/\4/"`
-
-				  if [ $ext = $base_ext ]; then
-					# Adding extension
-					ext_mask="("$ext_mask") | ("$ext_on" | "$ext_canon")"
-				  else
-					# Removing extension
-					ext_mask="("$ext_mask") & ~("$ext_off" | "$ext_canon")"
-				  fi
-
+				    || [ x"$opt_line" != x ]; then
 				  true
 				else
 				  echo "Unknown extension used in --with-$which=$val" 1>&2
@@ -4191,10 +4154,6 @@ case "${target}" in
 				ext_val=`echo $ext_val | sed -e 's/[a-z0-9]\+//'`
 			  done
 
-			  ext_mask="(("$ext_mask") << 6)"
-			  if [ x"$base_id" != x ]; then
-				target_cpu_cname="TARGET_CPU_$base_id | $ext_mask"
-			  fi
 			  true
 			else
 			  echo "Unknown $which used in --with-$which=$val" 1>&2
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 67ffba02d3e..c26263a3df8 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -1524,8 +1524,6 @@ static const struct attribute_spec aarch64_attribute_table[] =
   { NULL,                 0, 0, false, false, false, false, NULL, NULL }
 };
 
-#define AARCH64_CPU_DEFAULT_FLAGS ((selected_cpu) ? selected_cpu->flags : 0)
-
 /* An ISA extension in the co-processor and main instruction set space.  */
 struct aarch64_option_extension
 {
@@ -14870,35 +14868,24 @@ aarch64_validate_mtune (const char *str, const struct processor **res)
   return false;
 }
 
-/* Return the CPU corresponding to the enum CPU.
-   If it doesn't specify a cpu, return the default.  */
+/* Return the CPU corresponding to the enum CPU.  */
 
 static const struct processor *
 aarch64_get_tune_cpu (enum aarch64_processor cpu)
 {
-  if (cpu != aarch64_none)
-    return &all_cores[cpu];
+  gcc_assert (cpu != aarch64_none);
 
-  /* The & 0x3f is to extract the bottom 6 bits that encode the
-     default cpu as selected by the --with-cpu GCC configure option
-     in config.gcc.
-     ???: The whole TARGET_CPU_DEFAULT and AARCH64_CPU_DEFAULT_FLAGS
-     flags mechanism should be reworked to make it more sane.  */
-  return &all_cores[TARGET_CPU_DEFAULT & 0x3f];
+  return &all_cores[cpu];
 }
 
-/* Return the architecture corresponding to the enum ARCH.
-   If it doesn't specify a valid architecture, return the default.  */
+/* Return the architecture corresponding to the enum ARCH.  */
 
 static const struct processor *
 aarch64_get_arch (enum aarch64_arch arch)
 {
-  if (arch != aarch64_no_arch)
-    return &all_architectures[arch];
-
-  const struct processor *cpu = &all_cores[TARGET_CPU_DEFAULT & 0x3f];
+  gcc_assert (arch != aarch64_no_arch);
 
-  return &all_architectures[cpu->arch];
+  return &all_architectures[arch];
 }
 
 /* Return the VG value associated with -msve-vector-bits= value VALUE.  */
@@ -14936,10 +14923,6 @@ aarch64_override_options (void)
   uint64_t arch_isa = 0;
   aarch64_isa_flags = 0;
 
-  bool valid_cpu = true;
-  bool valid_tune = true;
-  bool valid_arch = true;
-
   selected_cpu = NULL;
   selected_arch = NULL;
   selected_tune = NULL;
@@ -14954,77 +14937,56 @@ aarch64_override_options (void)
      If either of -march or -mtune is given, they override their
      respective component of -mcpu.  */
   if (aarch64_cpu_string)
-    valid_cpu = aarch64_validate_mcpu (aarch64_cpu_string, &selected_cpu,
-					&cpu_isa);
+    aarch64_validate_mcpu (aarch64_cpu_string, &selected_cpu, &cpu_isa);
 
   if (aarch64_arch_string)
-    valid_arch = aarch64_validate_march (aarch64_arch_string, &selected_arch,
-					  &arch_isa);
+    aarch64_validate_march (aarch64_arch_string, &selected_arch, &arch_isa);
 
   if (aarch64_tune_string)
-    valid_tune = aarch64_validate_mtune (aarch64_tune_string, &selected_tune);
+    aarch64_validate_mtune (aarch64_tune_string, &selected_tune);
 
 #ifdef SUBTARGET_OVERRIDE_OPTIONS
   SUBTARGET_OVERRIDE_OPTIONS;
 #endif
 
-  /* If the user did not specify a processor, choose the default
-     one for them.  This will be the CPU set during configuration using
-     --with-cpu, otherwise it is "generic".  */
-  if (!selected_cpu)
-    {
-      if (selected_arch)
-	{
-	  selected_cpu = &all_cores[selected_arch->ident];
-	  aarch64_isa_flags = arch_isa;
-	  explicit_arch = selected_arch->arch;
-	}
-      else
-	{
-	  /* Get default configure-time CPU.  */
-	  selected_cpu = aarch64_get_tune_cpu (aarch64_none);
-	  aarch64_isa_flags = TARGET_CPU_DEFAULT >> 6;
-	}
-
-      if (selected_tune)
-	explicit_tune_core = selected_tune->ident;
-    }
-  /* If both -mcpu and -march are specified check that they are architecturally
-     compatible, warn if they're not and prefer the -march ISA flags.  */
-  else if (selected_arch)
+  if (selected_cpu && selected_arch)
     {
+      /* If both -mcpu and -march are specified, warn if they are not
+	 architecturally compatible and prefer the -march ISA flags.  */
       if (selected_arch->arch != selected_cpu->arch)
 	{
 	  warning (0, "switch %<-mcpu=%s%> conflicts with %<-march=%s%> switch",
 		       aarch64_cpu_string,
 		       aarch64_arch_string);
 	}
+
       aarch64_isa_flags = arch_isa;
-      explicit_arch = selected_arch->arch;
-      explicit_tune_core = selected_tune ? selected_tune->ident
-					  : selected_cpu->ident;
     }
-  else
+  else if (selected_cpu)
     {
-      /* -mcpu but no -march.  */
-      aarch64_isa_flags = cpu_isa;
-      explicit_tune_core = selected_tune ? selected_tune->ident
-					  : selected_cpu->ident;
-      gcc_assert (selected_cpu);
       selected_arch = &all_architectures[selected_cpu->arch];
-      explicit_arch = selected_arch->arch;
+      aarch64_isa_flags = cpu_isa;
     }
-
-  /* Set the arch as well as we will need it when outputing
-     the .arch directive in assembly.  */
-  if (!selected_arch)
+  else if (selected_arch)
     {
-      gcc_assert (selected_cpu);
+      selected_cpu = &all_cores[selected_arch->ident];
+      aarch64_isa_flags = arch_isa;
+    }
+  else
+    {
+      /* No -mcpu or -march specified, so use the default CPU.  */
+      selected_cpu = &all_cores[TARGET_CPU_DEFAULT];
       selected_arch = &all_architectures[selected_cpu->arch];
+      aarch64_isa_flags = selected_cpu->flags;
     }
 
+  explicit_arch = selected_arch->arch;
   if (!selected_tune)
     selected_tune = selected_cpu;
+  explicit_tune_core = selected_tune->ident;
+
+  gcc_assert (explicit_tune_core != aarch64_none);
+  gcc_assert (explicit_arch != aarch64_no_arch);
 
   if (aarch64_enable_bti == 2)
     {
@@ -15060,15 +15022,6 @@ aarch64_override_options (void)
   if (aarch64_ra_sign_scope != AARCH64_FUNCTION_NONE && TARGET_ILP32)
     sorry ("return address signing is only supported for %<-mabi=lp64%>");
 
-  /* Make sure we properly set up the explicit options.  */
-  if ((aarch64_cpu_string && valid_cpu)
-       || (aarch64_tune_string && valid_tune))
-    gcc_assert (explicit_tune_core != aarch64_none);
-
-  if ((aarch64_cpu_string && valid_cpu)
-       || (aarch64_arch_string && valid_arch))
-    gcc_assert (explicit_arch != aarch64_no_arch);
-
   /* The pass to insert speculation tracking runs before
      shrink-wrapping and the latter does not know how to update the
      tracking status.  So disable it in this case.  */
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index 0bdcc74ace5..cf7e2540842 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -775,8 +775,7 @@ enum target_cpus
 
 /* If there is no CPU defined at configure, use generic as default.  */
 #ifndef TARGET_CPU_DEFAULT
-#define TARGET_CPU_DEFAULT \
-  (TARGET_CPU_generic | (AARCH64_CPU_DEFAULT_FLAGS << 6))
+# define TARGET_CPU_DEFAULT TARGET_CPU_generic
 #endif
 
 /* If inserting NOP before a mult-accumulate insn remember to adjust the
-- 
2.25.1


[-- Attachment #4: 0003-AArch64-Add-support-for-with-tune.patch --]
[-- Type: application/octet-stream, Size: 6627 bytes --]

From 242284b1f408027a6fd8e72139149ac49396aae6 Mon Sep 17 00:00:00 2001
From: Wilco Dijkstra <wdijkstr@arm.com>
Date: Thu, 3 Dec 2020 18:40:34 +0000
Subject: [PATCH 3/3] AArch64: Add support for --with-tune

Add support for --with-tune. Like --with-cpu and --with-arch, the argument is
validated and transformed into a -mtune option to be processed like any other
command-line option.  --with-tune has no effect if a -mcpu or -mtune option
is used. The validating code didn't allow --with-cpu=native, so explicitly
allow that.

Co-authored-by:  Delia Burduv  <delia.burduv@arm.com>

Bootstrap OK, regress pass, OK to commit?

ChangeLog
2020-09-03  Wilco Dijkstra  <wdijkstr@arm.com>

        * config.gcc
        (aarch64*-*-*): Add --with-tune. Support --with-cpu=native.
        * config/aarch64/aarch64.h (OPTION_DEFAULT_SPECS): Add --with-tune.

2020-09-03  Wilco Dijkstra  <wdijkstr@arm.com>

        * gcc/testsuite/lib/target-supports.exp:
        (check_effective_target_tune_cortex_a76): New effective target test.
        * gcc.target/aarch64/with-tune-config.c: New test.
        * gcc.target/aarch64/with-tune-march.c: Likewise.
        * gcc.target/aarch64/with-tune-mcpu.c: Likewise.
        * gcc.target/aarch64/with-tune-mtune.c: Likewise.
---
 gcc/config.gcc                                    | 15 +++++++++++++--
 gcc/config/aarch64/aarch64.h                      | 10 ++++++----
 .../gcc.target/aarch64/with-tune-config.c         |  7 +++++++
 .../gcc.target/aarch64/with-tune-march.c          |  8 ++++++++
 gcc/testsuite/gcc.target/aarch64/with-tune-mcpu.c |  8 ++++++++
 .../gcc.target/aarch64/with-tune-mtune.c          |  7 +++++++
 gcc/testsuite/lib/target-supports.exp             |  5 +++++
 7 files changed, 54 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/with-tune-config.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/with-tune-march.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/with-tune-mcpu.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/with-tune-mtune.c

diff --git a/gcc/config.gcc b/gcc/config.gcc
index 7cdfc95ecee..afce813bc36 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -4121,6 +4121,12 @@ case "${target}" in
 				    ${srcdir}/config/aarch64/$def \
 				    > /dev/null; then
 
+			  # Disallow extensions in --with-tune=cortex-a53+crc.
+			  if [ $which = tune ] && [ x"$ext_val" != x ]; then
+			    echo "Architecture extensions not supported in --with-$which=$val" 1>&2
+			    exit 1
+			  fi
+
 			  # Use the pre-processor to strip flatten the options.
 			  # This makes the format less rigid than if we use
 			  # grep and sed directly here.
@@ -4156,8 +4162,13 @@ case "${target}" in
 
 			  true
 			else
-			  echo "Unknown $which used in --with-$which=$val" 1>&2
-			  exit 1
+			  # Allow --with-$which=native.
+			  if [ "$val" = native ]; then
+			    true
+			  else
+			    echo "Unknown $which used in --with-$which=$val" 1>&2
+			    exit 1
+			  fi
 			fi
 		done
 		;;
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index cf7e2540842..2d1d3528847 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -1211,12 +1211,14 @@ extern enum aarch64_code_model aarch64_cmodel;
 #define ENDIAN_LANE_N(NUNITS, N) \
   (BYTES_BIG_ENDIAN ? NUNITS - 1 - N : N)
 
-/* Support for a configure-time default CPU, etc.  We currently support
-   --with-arch and --with-cpu.  Both are ignored if either is specified
-   explicitly on the command line at run time.  */
+/* Support for configure-time --with-arch, --with-cpu and --with-tune.
+   --with-arch and --with-cpu are ignored if either -mcpu or -march is used.
+   --with-tune is ignored if either -mtune or -mcpu is used (but is not
+   affected by -march).  */
 #define OPTION_DEFAULT_SPECS				\
   {"arch", "%{!march=*:%{!mcpu=*:-march=%(VALUE)}}" },	\
-  {"cpu",  "%{!march=*:%{!mcpu=*:-mcpu=%(VALUE)}}" },
+  {"cpu",  "%{!march=*:%{!mcpu=*:-mcpu=%(VALUE)}}" },   \
+  {"tune", "%{!mcpu=*:%{!mtune=*:-mtune=%(VALUE)}}"},
 
 #define MCPU_TO_MARCH_SPEC \
    " %{mcpu=*:-march=%:rewrite_mcpu(%{mcpu=*:%*})}"
diff --git a/gcc/testsuite/gcc.target/aarch64/with-tune-config.c b/gcc/testsuite/gcc.target/aarch64/with-tune-config.c
new file mode 100644
index 00000000000..0940e9eea89
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/with-tune-config.c
@@ -0,0 +1,7 @@
+/* { dg-do compile { target { tune_cortex_a76 } } } */
+/* { dg-additional-options " -dA " } */
+
+void foo ()
+{}
+
+/* { dg-final { scan-assembler "//.tune cortex-a76" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/with-tune-march.c b/gcc/testsuite/gcc.target/aarch64/with-tune-march.c
new file mode 100644
index 00000000000..61039adea71
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/with-tune-march.c
@@ -0,0 +1,8 @@
+/* { dg-do compile { target { tune_cortex_a76 } } } */
+/* { dg-additional-options " -dA -march=armv8.6-a " } */
+
+void foo ()
+{}
+
+/* { dg-final { scan-assembler "//.tune cortex-a76" } } */
+/* { dg-final { scan-assembler ".arch armv8.6-a" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/with-tune-mcpu.c b/gcc/testsuite/gcc.target/aarch64/with-tune-mcpu.c
new file mode 100644
index 00000000000..4f8267a5c16
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/with-tune-mcpu.c
@@ -0,0 +1,8 @@
+/* { dg-do compile { target { tune_cortex_a76 } } } */
+/* { dg-additional-options " -dA -mcpu=cortex-a73" } */
+
+void foo ()
+{}
+
+/* { dg-final { scan-assembler "//.tune cortex-a73" } } */
+/* { dg-final { scan-assembler ".arch armv8-a" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/with-tune-mtune.c b/gcc/testsuite/gcc.target/aarch64/with-tune-mtune.c
new file mode 100644
index 00000000000..60f795a3919
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/with-tune-mtune.c
@@ -0,0 +1,7 @@
+/* { dg-do compile { target { tune_cortex_a76 } } } */
+/* { dg-additional-options " -dA -mtune=cortex-a73" } */
+
+void foo ()
+{}
+
+/* { dg-final { scan-assembler "//.tune cortex-a73" } } */
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 89c4f67554f..a4043f8677d 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -10591,6 +10591,11 @@ proc check_effective_target_msp430_large {} {
     } ""]
 }
 
+# Return 1 if GCC was configured with --with-tune=cortex-a76
+proc check_effective_target_tune_cortex_a76 { } {
+    return [check_configured_with "with-tune=cortex-a76"]
+}
+
 # Return 1 if the target has an efficient means to encode large initializers
 # in the assembly.
 
-- 
2.25.1


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

* Re: [AArch64] Add --with-tune configure flag
  2020-11-19 14:40   ` Wilco Dijkstra
@ 2020-11-19 16:52     ` Richard Earnshaw (lists)
  0 siblings, 0 replies; 15+ messages in thread
From: Richard Earnshaw (lists) @ 2020-11-19 16:52 UTC (permalink / raw)
  To: Wilco Dijkstra, Pop, Sebastian, GCC Patches; +Cc: Richard Sandiford

On 19/11/2020 14:40, Wilco Dijkstra via Gcc-patches wrote:
> Hi,
> 
>>>>     As for your second patch, --with-cpu-64 could be a simple alias indeed,
>>>>     but what is the exact definition/expected behaviour of a --with-cpu-32
>>>>     on a target that only supports 64-bit code? The AArch64 target cannot
>>>>     generate AArch32 code, so we shouldn't silently accept it.
>>>
>>> IMO allowing users to specify all the flags available on x86 is important.
>>>
>>
>> This isn't about general users though; it's about how you configure the
>> compiler and that's not all the same.  I don't mind the --with-cpu-64 as
>> a strict alias for --with-cpu, but --with-cpu-32 is both redundant and
>> misleading as it might give the impression that it does something useful.
> 
> We could make it do something useful, for example emit a warning, an error
> or default to -mabi=ilp32 (since that is similar to what other targets do).
> Anything is better than being the only target that doesn't support it...
> 
> Cheers,
> Wilco
> 

Having the same option have a completely different meaning would be even
worse than not having the option at all.  So no, that's a non-starter.

It's not like these configure options have wide-spread usage at present.

R.

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

* Re: [AArch64] Add --with-tune configure flag
  2020-11-19 10:25 ` Richard Earnshaw (lists)
@ 2020-11-19 14:40   ` Wilco Dijkstra
  2020-11-19 16:52     ` Richard Earnshaw (lists)
  0 siblings, 1 reply; 15+ messages in thread
From: Wilco Dijkstra @ 2020-11-19 14:40 UTC (permalink / raw)
  To: Richard Earnshaw, Pop, Sebastian, GCC Patches; +Cc: Richard Sandiford

Hi,

>>>    As for your second patch, --with-cpu-64 could be a simple alias indeed,
>>>    but what is the exact definition/expected behaviour of a --with-cpu-32
>>>    on a target that only supports 64-bit code? The AArch64 target cannot
>>>    generate AArch32 code, so we shouldn't silently accept it.
>> 
>> IMO allowing users to specify all the flags available on x86 is important.
>> 
>
> This isn't about general users though; it's about how you configure the
> compiler and that's not all the same.  I don't mind the --with-cpu-64 as
> a strict alias for --with-cpu, but --with-cpu-32 is both redundant and
> misleading as it might give the impression that it does something useful.

We could make it do something useful, for example emit a warning, an error
or default to -mabi=ilp32 (since that is similar to what other targets do).
Anything is better than being the only target that doesn't support it...

Cheers,
Wilco

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

* Re: [AArch64] Add --with-tune configure flag
  2020-11-18 17:16 Pop, Sebastian
@ 2020-11-19 10:25 ` Richard Earnshaw (lists)
  2020-11-19 14:40   ` Wilco Dijkstra
  0 siblings, 1 reply; 15+ messages in thread
From: Richard Earnshaw (lists) @ 2020-11-19 10:25 UTC (permalink / raw)
  To: Pop, Sebastian, Wilco Dijkstra, GCC Patches; +Cc: Richard Sandiford

On 18/11/2020 17:16, Pop, Sebastian via Gcc-patches wrote:
> Hi,
> 
> On 11/18/20, 10:17 AM, "Wilco Dijkstra" <Wilco.Dijkstra@arm.com> wrote:
>>    I presume you're trying to unify the --with- options across most targets?
> 
> Yes, my intention was to provide the same configure options on arm64
> as on x86, such that projects that already use those options can change
> cpu name to "neoverse-n1" and that will build a compiler with the right
> tuning for Graviton2.
> 
> Allowing arm64 users to specify all the flags available on x86 is important.
> 
>>    That would be very useful! However there are significant differences between
>>    targets in how they interpret options like --with-arch=native (or -march). So
>>    those differences also need to be looked at and fixed to avoid unexpected results.
>>
>>    As for the first patch, I think support for --witch-tune requires more changes.
>>    Without proper processing of a --with-tune, you get an incorrect architecture
>>    version (if say the CPU you tune for is newer than the --with-cpu/arch
>>    or default).
>>
>>   I posted patches to add --with-tune and fix various issues a while back:
>>    https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553865.html
>>    https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553866.html
> 
> Thanks for pointing me to your patches, I was not aware of these changes.
> I see that your patches enable more use cases and fix several bugs.
> These changes would definitely be good to have in trunk and branches.
> 
> My patch was the minimal change to enable --with-tune=neoverse-n1
> 
>>    As for your second patch, --with-cpu-64 could be a simple alias indeed,
>>    but what is the exact definition/expected behaviour of a --with-cpu-32
>>    on a target that only supports 64-bit code? The AArch64 target cannot
>>    generate AArch32 code, so we shouldn't silently accept it.
> 
> IMO allowing users to specify all the flags available on x86 is important.
> 

This isn't about general users though; it's about how you configure the
compiler and that's not all the same.  I don't mind the --with-cpu-64 as
a strict alias for --with-cpu, but --with-cpu-32 is both redundant and
misleading as it might give the impression that it does something useful.

R.

> Thanks,
> Sebastian
> 


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

* Re: [AArch64] Add --with-tune configure flag
@ 2020-11-18 17:16 Pop, Sebastian
  2020-11-19 10:25 ` Richard Earnshaw (lists)
  0 siblings, 1 reply; 15+ messages in thread
From: Pop, Sebastian @ 2020-11-18 17:16 UTC (permalink / raw)
  To: Wilco Dijkstra, GCC Patches
  Cc: Richard Sandiford, Richard Earnshaw, Kyrylo Tkachov, Jeff Law

Hi,

On 11/18/20, 10:17 AM, "Wilco Dijkstra" <Wilco.Dijkstra@arm.com> wrote:
>    I presume you're trying to unify the --with- options across most targets?

Yes, my intention was to provide the same configure options on arm64
as on x86, such that projects that already use those options can change
cpu name to "neoverse-n1" and that will build a compiler with the right
tuning for Graviton2.

Allowing arm64 users to specify all the flags available on x86 is important.

>    That would be very useful! However there are significant differences between
>    targets in how they interpret options like --with-arch=native (or -march). So
>    those differences also need to be looked at and fixed to avoid unexpected results.
>
>    As for the first patch, I think support for --witch-tune requires more changes.
>    Without proper processing of a --with-tune, you get an incorrect architecture
>    version (if say the CPU you tune for is newer than the --with-cpu/arch
>    or default).
>
>   I posted patches to add --with-tune and fix various issues a while back:
>    https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553865.html
>    https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553866.html

Thanks for pointing me to your patches, I was not aware of these changes.
I see that your patches enable more use cases and fix several bugs.
These changes would definitely be good to have in trunk and branches.

My patch was the minimal change to enable --with-tune=neoverse-n1

>    As for your second patch, --with-cpu-64 could be a simple alias indeed,
>    but what is the exact definition/expected behaviour of a --with-cpu-32
>    on a target that only supports 64-bit code? The AArch64 target cannot
>    generate AArch32 code, so we shouldn't silently accept it.

IMO allowing users to specify all the flags available on x86 is important.

Thanks,
Sebastian


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

* Re: [AArch64] Add --with-tune configure flag
@ 2020-11-18 16:16 Wilco Dijkstra
  0 siblings, 0 replies; 15+ messages in thread
From: Wilco Dijkstra @ 2020-11-18 16:16 UTC (permalink / raw)
  To: Pop, Sebastian, GCC Patches
  Cc: Richard Sandiford, Richard Earnshaw, Kyrylo Tkachov

Hi Sebastian,

I presume you're trying to unify the --with- options across most targets?
That would be very useful! However there are significant differences between
targets in how they interpret options like --with-arch=native (or -march). So
those differences also need to be looked at and fixed to avoid unexpected results.

As for the first patch, I think support for --witch-tune requires more changes.
Without proper processing of a --with-tune, you get an incorrect architecture
version (if say the CPU you tune for is newer than the --with-cpu/arch
or default).

I posted patches to add --with-tune and fix various issues a while back:

https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553865.html
https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553866.html

So I think these should go in first. They are simple and useful enough to backport
(since they fix bugs) but that decision is up to the AArch64 maintainers.

As for your second patch, --with-cpu-64 could be a simple alias indeed,
but what is the exact definition/expected behaviour of a --with-cpu-32
on a target that only supports 64-bit code? The AArch64 target cannot
generate AArch32 code, so we shouldn't silently accept it.

Cheers,
Wilco


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

end of thread, other threads:[~2020-12-10 19:03 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-17 17:53 [AArch64] Add --with-tune configure flag Pop, Sebastian
2020-11-17 21:27 ` Pop, Sebastian
2020-11-17 21:44   ` Jeff Law
2020-11-17 21:43 ` Jeff Law
2020-11-18 16:16 Wilco Dijkstra
2020-11-18 17:16 Pop, Sebastian
2020-11-19 10:25 ` Richard Earnshaw (lists)
2020-11-19 14:40   ` Wilco Dijkstra
2020-11-19 16:52     ` Richard Earnshaw (lists)
2020-12-04 16:34 Pop, Sebastian
2020-12-07 17:33 ` Richard Sandiford
2020-12-07 18:52   ` Wilco Dijkstra
2020-12-07 20:32     ` Richard Sandiford
2020-12-07 22:45       ` Wilco Dijkstra
2020-12-10 19:02       ` Wilco Dijkstra

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).