Re: [PATCHv3 4/7] arc: Remove EF_ARC_CPU_GENERIC constant.

[Andrew Burgess <andrew dot burgess at embecosm dot com>]

* Claudiu Zissulescu <Claudiu.Zissulescu@synopsys.com> [2016-03-16 11:23:36 +0000]:

> > No I only considered the GNU tools.  I will try to create a test and
> > see what happens, I may revise, or drop this patch from the series
> > then.
> 
> Yeah, I need to come up with a "compatibility" test here.  
> > 
> > Out of interest, how do you tell if an ELF is ARC700 / ARCEM / etc
> > when it is compiled with the official tools if not by using the
> > e_flags?  Is there some other field that should be used?
> > 
> The ARC700/ARC600 has a different machine than ARCHS/ARCEM. 

I hacked up a version of bfd that deliberately clears the
EF_ARC_MACH_MSK field in e_flags when creating ELFs, which I believe
should give results inline with the non-gnu synopsys arc compiler.

I've now tweaked the patch a little, and I looked at the differences
when viewing the ELF headers in readelf for and arc600, arc700, archs,
and arcem file all produced using the above hacked bfd.

With the revised patch the only change is that I've moved from
describing the files as "Generic" arc to "Unknown" arc.  I don't know
if you'll be happy with the change, but I think "Unknown" is a better
description of what we have.

To me generic means "would run on anything".  Now the machine type
does split the problem space in two, older and new architectures, but
I think the point about generic is still sound.

However, the each ELF might contains instructions that limits the file
to only being suitable for one, specific architecture, we just don't
know which one, I think "Unknown" reflects that situation better than
"Generic".

Commit message and patch below for your consideration.

Thanks,
Andrew

---

The constant EF_ARC_CPU_GENERIC is defined in the include/elf/arc.h
file, and is used in a few places in binutils, however, this constant
should never make it into the elf header flags; we always set a valid
cpu type in the assembler, which should then be copied over during
linking.

There are some non-gnu arc compilers that don't write an architecture
type into the e_flags field, instead leaving the field as 0, which is
the EF_ARC_CPU_GENERIC value.  This non-gnu compiler uses the machine
type to distinguish between the old and newer arc architectures, setting
the machine type to EM_ARC_COMPACT for old arc600, arc601, and arc700
architectures, while using EM_ARC_COMPACT2 for newer arcem and archs
architectures.

Previously when displaying the machine flags for an older EM_ARC_COMPACT
machine, if the e_flags had not been filled in, then we relied on the
default case statement to display the message "Generic ARCompact", while
in the EM_ARC_COMPACT2 case we specifically handled EF_ARC_CPU_GENERIC
to print "ARC Generic", leaving the default case to print a message
about unrecognised cpu flag.

After this commit EF_ARC_CPU_GENERIC has been removed, for both machine
types EM_ARC_COMPACT and EM_ARC_COMPACT2 we now rely on the default case
statement to handle the situation where the e_flags has not been filled
in.  The message displayed is now "Unknown ARCompact" (for older arc
architectures) and "Unknown ARC" (for the newer architectures).  The
switch from "Generic" to "Unknown" in the message string is for clarity,
calling the file "Generic" can give the impression that the file is
compiled for a common sub-set of the architectures, and would therefore
run on any type of machine (or at least any type of new or old machine
depending on if the machine type is ARC or ARCv2).  However, this was
not what "Generic" meant, it really meant "Unknown", so that's what we
now say.

As part of the merging of the readelf flag reading code, I have unified
the strings used in displaying the ELF ABI.  This means that for older
arc machines (arc600, arc601, and arc700) the string used for the
original ABI, and ABIv2 have changed, the current ABIv3 remains the
same.  For the newer architectures (arcem and archs) the abi strings
remain unchanged in all cases.

bfd/ChangeLog:

	* elf32-arc.c (arc_elf_print_private_bfd_data): Remove use of
	EF_ARC_CPU_GENERIC.
	(arc_elf_final_write_processing): Don't bother setting cpu field
	in e_flags, this will have been set elsewhere.

binutils/ChangeLog:

	* readelf.c (get_machine_flags): Move arc processing into...
	(decode_ARC_machine_flags): ... new function.  Remove use of
	EF_ARC_CPU_GENERIC, change default case from "generic arc" to
	"unknown arc".  Merged ABI printing between two machine types.

gas/ChangeLog:

	* config/tc-arc.c (arc_select_cpu): Remove use of
	EF_ARC_CPU_GENERIC.

include/ChangeLog:

	* elf/arc.h (EF_ARC_CPU_GENERIC): Delete.  Update related comment.
---
 bfd/ChangeLog       |   7 +++
 bfd/elf32-arc.c     |  10 ----
 binutils/ChangeLog  |   7 +++
 binutils/readelf.c  | 143 +++++++++++++++++++++++++---------------------------
 gas/ChangeLog       |   5 ++
 gas/config/tc-arc.c |   7 ++-
 include/ChangeLog   |   4 ++
 include/elf/arc.h   |   6 +--
 8 files changed, 97 insertions(+), 92 deletions(-)

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index d2532f0..20c1904 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,5 +1,12 @@
 2016-03-15  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* elf32-arc.c (arc_elf_print_private_bfd_data): Remove use of
+	EF_ARC_CPU_GENERIC.
+	(arc_elf_final_write_processing): Don't bother setting cpu field
+	in e_flags, this will have been set elsewhere.
+
+2016-03-15  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* elf32-arc.c (arc_elf_final_write_processing): Switch to using
 	EF_ARC_MACH_MSK.
 
diff --git a/bfd/elf32-arc.c b/bfd/elf32-arc.c
index 7c856d0..ec81852 100644
--- a/bfd/elf32-arc.c
+++ b/bfd/elf32-arc.c
@@ -415,7 +415,6 @@ arc_elf_print_private_bfd_data (bfd *abfd, void * ptr)
 
   switch (flags & EF_ARC_MACH_MSK)
     {
-    case EF_ARC_CPU_GENERIC : fprintf (file, " -mcpu=generic"); break;
     case EF_ARC_CPU_ARCV2HS : fprintf (file, " -mcpu=ARCv2HS");    break;
     case EF_ARC_CPU_ARCV2EM : fprintf (file, " -mcpu=ARCv2EM");    break;
     case E_ARC_MACH_ARC600  : fprintf (file, " -mcpu=ARC600");     break;
@@ -647,34 +646,25 @@ static void
 arc_elf_final_write_processing (bfd * abfd,
 				bfd_boolean linker ATTRIBUTE_UNUSED)
 {
-  unsigned long val;
   unsigned long emf;
 
   switch (bfd_get_mach (abfd))
     {
     case bfd_mach_arc_arc600:
-      val = E_ARC_MACH_ARC600;
       emf = EM_ARC_COMPACT;
       break;
     case bfd_mach_arc_arc601:
-      val = E_ARC_MACH_ARC601;
       emf = EM_ARC_COMPACT;
       break;
     case bfd_mach_arc_arc700:
-      val = E_ARC_MACH_ARC700;
       emf = EM_ARC_COMPACT;
       break;
     case bfd_mach_arc_arcv2:
-      val = EF_ARC_CPU_GENERIC;
       emf = EM_ARC_COMPACT2;
-      /* TODO: Check validity of this.  It can also be ARCV2EM here.
-	 Previous version sets the e_machine here.  */
       break;
     default:
       abort ();
     }
-  if ((elf_elfheader (abfd)->e_flags & EF_ARC_MACH_MSK) == EF_ARC_CPU_GENERIC)
-    elf_elfheader (abfd)->e_flags |= val;
 
   elf_elfheader (abfd)->e_machine = emf;
 
diff --git a/binutils/ChangeLog b/binutils/ChangeLog
index 9328815..f96ee49 100644
--- a/binutils/ChangeLog
+++ b/binutils/ChangeLog
@@ -1,5 +1,12 @@
 2016-03-15  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* readelf.c (get_machine_flags): Move arc processing into...
+	(decode_ARC_machine_flags): ... new function.  Remove use of
+	EF_ARC_CPU_GENERIC, change default case from "generic arc" to
+	"unknown arc".  Merged ABI printing between two machine types.
+
+2016-03-15  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* testsuite/binutils-all/objdump.exp (cpus_expected): Add ARC700
 	to the architecture list.
 
diff --git a/binutils/readelf.c b/binutils/readelf.c
index 7deac04..fe23d45 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -2271,6 +2271,73 @@ get_machine_name (unsigned e_machine)
 }
 
 static void
+decode_ARC_machine_flags (unsigned e_flags, unsigned e_machine, char buf[])
+{
+  /* ARC has two machine types EM_ARC_COMPACT and EM_ARC_COMPACT2.  Some
+     other compilers don't a specific architecture type in the e_flags, and
+     instead use EM_ARC_COMPACT for old ARC600, ARC601, and ARC700
+     architectures, and switch to EM_ARC_COMPACT2 for newer ARCEM and ARCHS
+     architectures.
+
+     Th GNU tools follows this use of EM_ARC_COMPACT and EM_ARC_COMPACT2,
+     but also sets a specific architecture type in the e_flags field.
+
+     However, when decoding the flags we don't worry if we see an
+     unexpected pairing, for example EM_ARC_COMPACT machine type, with
+     ARCEM architecture type.  */
+
+  switch (e_flags & EF_ARC_MACH_MSK)
+    {
+      /* We only expect these to occur for EM_ARC_COMPACT2.  */
+    case EF_ARC_CPU_ARCV2EM:
+      strcat (buf, ", ARC EM");
+      break;
+    case EF_ARC_CPU_ARCV2HS:
+      strcat (buf, ", ARC HS");
+      break;
+
+      /* We only expect these to occur for EM_ARC_COMPACT.  */
+    case E_ARC_MACH_ARC600:
+      strcat (buf, ", ARC600");
+      break;
+    case E_ARC_MACH_ARC601:
+      strcat (buf, ", ARC601");
+      break;
+    case E_ARC_MACH_ARC700:
+      strcat (buf, ", ARC700");
+      break;
+
+      /* The only times we should end up here are (a) A corrupt ELF, (b) A
+         new ELF with new architecture being read by an old version of
+         readelf, or (c) An ELF built with non-GNU compiler that does not
+         set the architecture in the e_flags.  */
+    default:
+      if (e_machine == EM_ARC_COMPACT)
+        strcat (buf, ", Unknown ARCompact");
+      else
+        strcat (buf, ", Unknown ARC");
+      break;
+    }
+
+  switch (e_flags & EF_ARC_OSABI_MSK)
+    {
+    case E_ARC_OSABI_ORIG:
+      strcat (buf, ", (ABI:legacy)");
+      break;
+    case E_ARC_OSABI_V2:
+      strcat (buf, ", (ABI:v2)");
+      break;
+      /* Only upstream 3.9+ kernels will support ARCv2 ISA.  */
+    case E_ARC_OSABI_V3:
+      strcat (buf, ", v3 no-legacy-syscalls ABI");
+      break;
+    default:
+      strcat (buf, ", unrecognised ARC OSABI flag");
+      break;
+    }
+}
+
+static void
 decode_ARM_machine_flags (unsigned e_flags, char buf[])
 {
   unsigned eabi;
@@ -2768,81 +2835,9 @@ get_machine_flags (unsigned e_flags, unsigned e_machine)
 	  break;
 
 	case EM_ARC_COMPACT2:
-	  switch (e_flags & EF_ARC_MACH_MSK)
-	    {
-	    case EF_ARC_CPU_ARCV2EM:
-	      strcat (buf, ", ARC EM");
-	      break;
-	    case EF_ARC_CPU_ARCV2HS:
-	      strcat (buf, ", ARC HS");
-	      break;
-	    case EF_ARC_CPU_GENERIC:
-	      strcat (buf, ", ARC generic");
-	      break;
-	    case E_ARC_MACH_ARC600:
-	      strcat (buf, ", ARC600");
-	      break;
-	    case E_ARC_MACH_ARC601:
-	      strcat (buf, ", ARC601");
-	      break;
-	    case E_ARC_MACH_ARC700:
-	      strcat (buf, ", ARC700");
-	      break;
-	    default:
-	      strcat (buf, ", unrecognized cpu flag for ARCv2");
-	      break;
-	    }
-	  switch (e_flags & EF_ARC_OSABI_MSK)
-	    {
-	    case E_ARC_OSABI_ORIG:
-	      strcat (buf, ", (ABI:legacy)");
-	      break;
-	    case E_ARC_OSABI_V2:
-	      strcat (buf, ", (ABI:v2)");
-	      break;
-	      /* Only upstream 3.9+ kernels will support ARCv2 ISA.  */
-	    case E_ARC_OSABI_V3:
-	      strcat (buf, ", v3 no-legacy-syscalls ABI");
-	      break;
-	    default:
-	      strcat (buf, ", unrecognised ARC OSABI flag");
-	      break;
-	    }
-	  break;
-
 	case EM_ARC_COMPACT:
-	  switch (e_flags & EF_ARC_MACH_MSK)
-	    {
-	    case E_ARC_MACH_ARC600:
-	      strcat (buf, ", ARC 600");
-	      break;
-	    case E_ARC_MACH_ARC601:
-	      strcat (buf, ", ARC 601");
-	      break;
-	    case E_ARC_MACH_ARC700:
-	      strcat (buf, ", ARC 700");
-	      break;
-	    default:
-	      strcat (buf, ", Generic ARCompact");
-	      break;
-	    }
-	  switch (e_flags & EF_ARC_OSABI_MSK)
-	    {
-	    case E_ARC_OSABI_ORIG:
-	      strcat (buf, ", legacy syscall ABI");
-	      break;
-	    case E_ARC_OSABI_V2:
-	      /* For 3.2+ Linux kernels which use asm-generic
-		 hdrs.  */
-	      strcat (buf, ", v2 syscall ABI");
-	      break;
-	    case E_ARC_OSABI_V3:
-	      /* Upstream 3.9+ kernels which don't use any legacy
-		 syscalls.  */
-	      strcat (buf, ", v3 no-legacy-syscalls ABI");
-	      break;
-	    }
-	  break;
+          decode_ARC_machine_flags (e_flags, e_machine, buf);
+          break;
 
 	case EM_ARM:
 	  decode_ARM_machine_flags (e_flags, buf);
diff --git a/gas/ChangeLog b/gas/ChangeLog
index fc026ea..16d7f34 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,5 +1,10 @@
 2016-03-15  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* config/tc-arc.c (arc_select_cpu): Remove use of
+	EF_ARC_CPU_GENERIC.
+
+2016-03-15  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* config/tc-arc.c (arc_target): Delay initialisation until
 	arc_select_cpu.
 	(arc_target_name): Likewise.
diff --git a/gas/config/tc-arc.c b/gas/config/tc-arc.c
index 5633905..65eb0e9 100644
--- a/gas/config/tc-arc.c
+++ b/gas/config/tc-arc.c
@@ -573,7 +573,7 @@ md_number_to_chars_midend (char *buf, valueT val, int n)
 static void
 arc_select_cpu (const char *arg)
 {
-  int cpu_flags = EF_ARC_CPU_GENERIC;
+  int cpu_flags = 0;
   int i;
 
   for (i = 0; cpu_types[i].name; ++i)
@@ -591,9 +591,8 @@ arc_select_cpu (const char *arg)
 
   if (!cpu_types[i].name)
     as_fatal (_("unknown architecture: %s\n"), arg);
-
-  if (cpu_flags != EF_ARC_CPU_GENERIC)
-    arc_eflag = (arc_eflag & ~EF_ARC_MACH_MSK) | cpu_flags;
+  gas_assert (cpu_flags != 0);
+  arc_eflag = (arc_eflag & ~EF_ARC_MACH_MSK) | cpu_flags;
 }
 
 /* Here ends all the ARCompact extension instruction assembling
diff --git a/include/ChangeLog b/include/ChangeLog
index b083a8d..102d1c0 100644
--- a/include/ChangeLog
+++ b/include/ChangeLog
@@ -1,5 +1,9 @@
 2016-03-15  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* elf/arc.h (EF_ARC_CPU_GENERIC): Delete.  Update related comment.
+
+2016-03-15  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* elf/arc.h (EF_ARC_MACH): Delete.
 	(EF_ARC_MACH_MSK): Remove out of date comment.
 
diff --git a/include/elf/arc.h b/include/elf/arc.h
index 0b75465..47381f3 100644
--- a/include/elf/arc.h
+++ b/include/elf/arc.h
@@ -43,13 +43,11 @@ END_RELOC_NUMBERS (R_ARC_max)
 #define EF_ARC_OSABI_MSK 0x00000f00
 #define EF_ARC_ALL_MSK	 (EF_ARC_MACH_MSK | EF_ARC_OSABI_MSK)
 
-/* Various CPU types.  */
+/* Various CPU types.  These numbers are exposed in the ELF header flags
+   (e_flags field), and so must never change.  */
 #define E_ARC_MACH_ARC600	0x00000002
 #define E_ARC_MACH_ARC601	0x00000004
 #define E_ARC_MACH_ARC700	0x00000003
-
-/* Processor specific flags for the ELF header e_flags field.  */
-#define EF_ARC_CPU_GENERIC      0x00000000
 #define EF_ARC_CPU_ARCV2EM      0x00000005
 #define EF_ARC_CPU_ARCV2HS      0x00000006
 
-- 
2.5.1