public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
From: Tamar Christina <tnfchris@gcc.gnu.org>
To: gcc-cvs@gcc.gnu.org
Subject: [gcc r14-5675] AArch64: only emit mismatch error when features would be disabled.
Date: Tue, 21 Nov 2023 13:26:00 +0000 (GMT)	[thread overview]
Message-ID: <20231121132600.7E879385B513@sourceware.org> (raw)

https://gcc.gnu.org/g:da332ce109451c8965cb64847934da154d7dcf94

commit r14-5675-gda332ce109451c8965cb64847934da154d7dcf94
Author: Tamar Christina <tamar.christina@arm.com>
Date:   Tue Nov 21 13:21:39 2023 +0000

    AArch64: only emit mismatch error when features would be disabled.
    
    At the moment we emit a warning whenever you specify both -march and -mcpu
    and the architecture of them differ.  The idea originally was that the user may
    not be aware of this change.
    
    However this has a few problems:
    
    1.  Architecture revisions is not an observable part of the architecture,
        extensions are.  Starting with GCC 14 we have therefore relaxed the rule that
        all extensions can be enabled at any architecture level.  Therefore it's
        incorrect, or at least not useful to keep the check on architecture.
    
    2.  It's problematic in Makefiles and other build systems, where you want to
        for certain files enable CPU specific builds.  i.e. you may be by default
        building for -march=armv8-a but for some file for -mcpu=neoverse-n1.  Since
        there's no easy way to remove the earlier options we end up warning and
        there's no way to disable just this warning.  Build systems compiling with
        -Werror face an issue in this case that compiling with GCC is needlessly
        hard.
    
    3. It doesn't actually warn for cases that may lead to issues, so e.g.
       -march=armv8.2-a+sve -mcpu=neoverse-n1 does not give a warning that SVE would
       be disabled.
    
    For this reason I have one of two proposals:
    
    1.  Just remove this warning all together.
    
    2.  Rework the warning based on extensions and only warn when features would be
        disabled by the presence of the -mcpu.  This is the approach this patch has
        taken.
    
    As examples:
    
    > aarch64-none-linux-gnu-gcc -march=armv8.2-a+sve -mcpu=neoverse-n1
    cc1: warning: switch ‘-mcpu=neoverse-n1’ conflicts with ‘-march=armv8.2-a+sve’ switch and resulted in options +crc+sve+norcpc+nodotprod being added                                                                                                                                        .arch armv8.2-a+crc+sve
    
    > aarch64-none-linux-gnu-gcc -march=armv8.2-a -mcpu=neoverse-n1
    > aarch64-none-linux-gnu-gcc -march=armv8.2-a+dotprod -mcpu=neoverse-n1
    > aarch64-none-linux-gnu-gcc -march=armv8.2-a+dotprod -mcpu=neoverse-n2
    <no warning>
    
    The one remaining issue here is that if both -march and -mcpu are specified we
    pick the -march.  This is not particularly obvious and for the use case to be
    more useful I think it makes sense to pick the CPU's arch?
    
    I did not make that change in the patch as it changes semantics.
    
    Note that I can't write a test for this because dg-warning expects warnings to
    be at a particular line and doesn't support warnings at the "global" level.
    
    gcc/ChangeLog:
    
            * config/aarch64/aarch64.cc (aarch64_override_options): Rework warnings.

Diff:
---
 gcc/config/aarch64/aarch64.cc | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index c4117c6c917..f6f6f94bf43 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -16436,12 +16436,22 @@ aarch64_override_options (void)
   if (cpu && arch)
     {
       /* If both -mcpu and -march are specified, warn if they are not
-	 architecturally compatible and prefer the -march ISA flags.  */
-      if (arch->arch != cpu->arch)
-	{
-	  warning (0, "switch %<-mcpu=%s%> conflicts with %<-march=%s%> switch",
+	 feature compatible.  feature compatible means that the inclusion of the
+	 cpu features would end up disabling an achitecture feature.  In
+	 otherwords the cpu features need to be a strict superset of the arch
+	 features and if so prefer the -march ISA flags.  */
+      auto full_arch_flags = arch->flags | arch_isa;
+      auto full_cpu_flags = cpu->flags | cpu_isa;
+      if (~full_cpu_flags & full_arch_flags)
+	{
+	  std::string ext_diff
+	    = aarch64_get_extension_string_for_isa_flags (full_arch_flags,
+							  full_cpu_flags);
+	  warning (0, "switch %<-mcpu=%s%> conflicts with %<-march=%s%> switch "
+		      "and resulted in options %<%s%> being added",
 		       aarch64_cpu_string,
-		       aarch64_arch_string);
+		       aarch64_arch_string,
+		       ext_diff.c_str ());
 	}
 
       selected_arch = arch->arch;

                 reply	other threads:[~2023-11-21 13:26 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20231121132600.7E879385B513@sourceware.org \
    --to=tnfchris@gcc.gnu.org \
    --cc=gcc-cvs@gcc.gnu.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).