[PATCH] D88390: [M68K] (Patch 4/8) MC layer and object file support

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added inline comments.


================
Comment at: llvm/include/llvm/Object/ELFObjectFile.h:1094
+    case ELF::EM_68K:
+      return "ELF32-M680x0";
     case ELF::EM_386:
----------------
glaubitz wrote:
> jrtc27 wrote:
> > glaubitz wrote:
> > > jrtc27 wrote:
> > > > As I said on D88389:
> > > > 
> > > > This gets reported in the file format line of llvm-objdump so should match what binutils has, which is elf32-m68k, though even if that weren't the case it should at least be in keeping with the style of all the others here.
> > > Yeah, I agree this should definitely match with what GNU is using there.
> > > 
> > > I would still prefer the backend being called "M680x0" and therefore the patches should be prefixed with "[M680x0]", similar to "SystemZ" and "s390x".
> > > 
> > > Naming the "M680x0" instead of "M68K" improves the readability in my personal opinion as it's easier to tell when you are talking about the backend and when you're talking about the architecture and GNU triplet.
> > The difference there is "IBM System z9" etc are the actual product names, and it's not a really clumsy name to type like M680x0. NXP's own site categorises its M68K-derived processors as "68K Processors (Legacy)", and the manuals say things like:
> > 
> > > The MCF5407 extends the legacy of Motorola’s 68K family
> > 
> > GCC's own manage uses the M680x0 term in the following ways:
> > 
> > > These are the -m options defined for M680x0 and ColdFire processors.
> > 
> > > Generate code for a specific M680x0 or ColdFire instruction set architecture. Permissible values of arch for M680x0 architectures are: 68000, 68010, 68020, 68030, 68040, 68060 and cpu32.  ColdFire architectures are selected according to Freescale's ISA classification and the permissible values are: isaa, isaaplus, isab and isac.
> > 
> > So the name M680x0 would actually be *more* narrow than what M68K means in practice, with the latter being the general term for any M68000-derived processor and the former being only for the 68000 through 68060 processors and *not* including the ColdFire extensions, yet there's no reason why our backend can't support that just like GCC does.
> Meh, I think this is really a bike-shedding contest. As I said, I like the name "M680x0" because it clearly tells me we're talking about the backend and not the architecture. It makes reading the code easier in my opinion.
> 
> > GCC's own manage uses the M680x0 term in the following ways:
> 
> Which is titled with "3.19.25 M680x0 Options" ;-)
> 
> See: https://gcc.gnu.org/onlinedocs/gcc/M680x0-Options.html
> 
> 
> Which is titled with "3.19.25 M680x0 Options" ;-)

Which I think is a bug (perhaps historical, if that was the name chosen before ColdFire was added) given the language used throughout. I've been considering sending a patch to change that title though.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88390/new/

https://reviews.llvm.org/D88390