[cfe][M68k] (Patch 7/8) Basic Clang support

[myhsu]

Summary

1. Add M68k as new Clang target
2. Add new attribute to support M68k's ISR (Interrupt Service Routine)

[myhsu]

Fix formatting issues

[myhsu]

Update licenses

[myhsu]

Fix the CPU name passing to the backend. Also removed M68060 for now, since backend hasn't fully implemented it yet.

[myhsu]

[NFC] Rename M680x0 to M68k

[rengolin]

clang/lib/Basic/Targets.cpp

314

No support for bare-metal?

clang/lib/Basic/Targets/M68k.cpp

124

no "sp", "pc", etc? Or are all of them aliased to those?

clang/lib/CodeGen/TargetInfo.cpp

8085

That's an odd comment... :)

What doesn't work right now? Something or everything?

[RKSimon]

clang/lib/Basic/CMakeLists.txt

82

(sorting) - move after Le64.cpp

clang/lib/CodeGen/TargetInfo.cpp

8089

(style) Use const auto * for cast results

[bruno]

Looking forward to see m68k support (and hopefully sega genesis toolchain support someday)!

clang/lib/Basic/Targets/M68k.cpp

74

This doesn't seem to match the coding style, clang-format to the rescue.

90

Can you make all the defineMacro inline within the cases? Clarity seems more useful here.

clang/lib/Basic/Targets/M68k.h

36

Can you remove the comment and add the enum back whenever you introduce CK_68060 support?

[myhsu]

• Rebased to latest changes
• Addressed some of the feedbacks

[myhsu]

• Addressed all the feedbacks
• Fixed minor issues that would retrieve the wrong TargetCodeGenInfo instance

clang/lib/Basic/Targets.cpp

314

currently we don't have any plan for that

clang/lib/Basic/Targets/M68k.cpp

124

stack pointer is aliased to a7, but I'll add pc into the list.

clang/lib/CodeGen/TargetInfo.cpp

8085

Seems to work after I fixed a minor issue in getting the correct TargetCodeGenInfo. Will remove this comment.

[rengolin]

Thanks for the changes. This looks good to me but I'll let others check again and approve.

[myhsu]

• Add support for M68060 sub-target

[bruno]

LGTM

[RKSimon]

clang/lib/Basic/Targets/M68k.h

15

@myhsu This should probably be LLVM_CLANG_LIB_BASIC_TARGETS_M68K_H

[jrtc27]

clang/lib/Basic/Targets/M68k.cpp

39–51

If we're in SysV psABI land, then the stack is 32-bit aligned.

If we're in actual ABI used by everyone out there, i.e. GCC's default, then it's only 16-bit aligned, and your integer types also have the wrong alignment, so you will get ABI incompatibility with GCC-built binaries as provided by distributions like Debian.

64

GCC's -mcpu excludes any M prefix and is just the number.

78–80

Where are these coming from? GCC only defines __m68k__.

[myhsu]

• Addressed feedbacks
  • Now M68k tries to use ABI that is compatible with GCC

clang/lib/Basic/Targets/M68k.cpp

64

we also support -mcpu with just number via m68k::getM68kTargetCPU

78–80

I think GCC does: https://github.com/gcc-mirror/gcc/blob/b90d051ecbc1d8972ae1bf0cd7588fcc66df0722/gcc/config/m68k/m68k.h#L64

Also I found this page: https://sourceforge.net/p/predef/wiki/Architectures

[jrtc27]

clang/lib/Basic/Targets/M68k.cpp

78–80

Yes, and as you can see none of those three are defined by GCC. Defining M68k is particularly bad as it will collide with the C++ namespace used by this very backend so you won't be able to build Clang with Clang on m68k! And the others are just a waste of time. Defining fewer macros is best, that way people don't come to rely on non-standard ones and instead use the single option that exists everywhere (__m68k__).

[myhsu]

• Addressed feedback
  • Remove redundant target macro definitions

clang/lib/Basic/Targets/M68k.cpp

78–80

it will collide with the C++ namespace used by this very backend so you won't be able to build Clang with Clang on m68k

good point :-)

Defining fewer macros is best, that way people don't come to rely on non-standard ones and instead use the single option that exists everywhere

fair enough

[jrtc27]

clang/include/clang/Basic/Attr.td

1541

Do you need to add M68kInterrupt to this list, including for all the other copies of it?

clang/lib/Basic/Targets/M68k.cpp

143

This is implemented?

165

Use assert/llvm_unreachable if this is wrong

clang/lib/Basic/Targets/M68k.h

58

(not even the right guard name)

[jrtc27]

Issues from my previous review are still outstanding

[myhsu]

• [NFC] Addressed feedbacks

[myhsu]

clang/lib/Basic/Targets/M68k.cpp

143

only part of the constraints are implements for now. I edit the FIXME comment and move to the end of switch statement

clang/include/clang/Basic/Attr.td

 def TargetARM : TargetArch<["arm", "thumb", "armeb", "thumbeb"]>;
 def TargetAArch64 : TargetArch<["aarch64"]>;
 def TargetAnyArm : TargetArch<!listconcat(TargetARM.Arches, TargetAArch64.Arches)>;
 def TargetAVR : TargetArch<["avr"]>;
 def TargetBPF : TargetArch<["bpfel", "bpfeb"]>;
 def TargetMips32 : TargetArch<["mips", "mipsel"]>;
 def TargetAnyMips : TargetArch<["mips", "mipsel", "mips64", "mips64el"]>;
 def TargetMSP430 : TargetArch<["msp430"]>;
+def TargetM68k : TargetArch<["m68k"]>;
 def TargetRISCV : TargetArch<["riscv32", "riscv64"]>;
 def TargetX86 : TargetArch<["x86"]>;
 def TargetAnyX86 : TargetArch<["x86", "x86_64"]>;
 def TargetWebAssembly : TargetArch<["wasm32", "wasm64"]>;
 def TargetHasDLLImportExport : TargetSpec {
   let CustomCode = [{ Target.getTriple().hasDLLImportExport() }];
 }
 def TargetItaniumCXXABI : TargetSpec {

 }
 
 def MipsShortCall : InheritableAttr, TargetSpecificAttr<TargetAnyMips> {
   let Spellings = [GCC<"short_call">, GCC<"near">];
   let Subjects = SubjectList<[Function]>;
   let Documentation = [MipsShortCallStyleDocs];
 }
 
+def M68kInterrupt : InheritableAttr, TargetSpecificAttr<TargetM68k> {
+  // NOTE: If you add any additional spellings, ARMInterrupt's, MipsInterrupt's
+  // and AnyX86Interrupt's spellings must match.
+  let Spellings = [GNU<"interrupt">];
+  let Args = [UnsignedArgument<"Number">];
+  let ParseKind = "Interrupt";
+  let HasCustomParsing = 1;
+  let Documentation = [Undocumented];
+}
+
 def Mode : Attr {
   let Spellings = [GCC<"mode">];
   let Subjects = SubjectList<[Var, Enum, TypedefName, Field], ErrorDiag>;
   let Args = [IdentifierArgument<"Mode">];
   let Documentation = [Undocumented];
   // This is notionally a type attribute, which #pragma clang attribute
   // generally does not support.
   let PragmaAttributeSupport = 0;

clang/lib/Basic/CMakeLists.txt

add_clang_library(clangBasic
   Targets/AMDGPU.cpp
   Targets/ARC.cpp
   Targets/ARM.cpp
   Targets/AVR.cpp
   Targets/BPF.cpp
   Targets/Hexagon.cpp
   Targets/Lanai.cpp
   Targets/Le64.cpp
+  Targets/M68k.cpp
   Targets/MSP430.cpp
   Targets/Mips.cpp
   Targets/NVPTX.cpp
   Targets/OSTargets.cpp
   Targets/PNaCl.cpp
   Targets/PPC.cpp
   Targets/RISCV.cpp
   Targets/SPIR.cpp
   Targets/Sparc.cpp
   Targets/SystemZ.cpp
   Targets/TCE.cpp

clang/lib/Basic/Targets.cpp

 #include "Targets/AMDGPU.h"
 #include "Targets/ARC.h"
 #include "Targets/ARM.h"
 #include "Targets/AVR.h"
 #include "Targets/BPF.h"
 #include "Targets/Hexagon.h"
 #include "Targets/Lanai.h"
 #include "Targets/Le64.h"
+#include "Targets/M68k.h"
 #include "Targets/MSP430.h"
 #include "Targets/Mips.h"
 #include "Targets/NVPTX.h"
 #include "Targets/OSTargets.h"
 #include "Targets/PNaCl.h"
 #include "Targets/PPC.h"
 #include "Targets/RISCV.h"
 #include "Targets/SPIR.h"
case llvm::Triple::mips64el:
     case llvm::Triple::NetBSD:
       return new NetBSDTargetInfo<MipsTargetInfo>(Triple, Opts);
     case llvm::Triple::OpenBSD:
       return new OpenBSDTargetInfo<MipsTargetInfo>(Triple, Opts);
     default:
       return new MipsTargetInfo(Triple, Opts);
     }
 
+  case llvm::Triple::m68k:
+    switch (os) {
+    case llvm::Triple::Linux:
+      return new LinuxTargetInfo<M68kTargetInfo>(Triple, Opts);
+    case llvm::Triple::NetBSD:
+      return new NetBSDTargetInfo<M68kTargetInfo>(Triple, Opts);
+    default:
+      return new M68kTargetInfo(Triple, Opts);
+    }
+
   case llvm::Triple::le32:
     switch (os) {
     case llvm::Triple::NaCl:
       return new NaClTargetInfo<PNaClTargetInfo>(Triple, Opts);
     default:
       return nullptr;
     }
 

clang/lib/Basic/Targets/M68k.h

+//===--- M68k.h - Declare M68k target feature support -------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file declares M68k TargetInfo objects.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef M680X0_H_LTNCIPAD
+#define M680X0_H_LTNCIPAD
+
+#include "OSTargets.h"
+#include "clang/Basic/TargetInfo.h"
+#include "clang/Basic/TargetOptions.h"
+#include "llvm/ADT/Triple.h"
+#include "llvm/Support/Compiler.h"
+
+namespace clang {
+namespace targets {
+
+class LLVM_LIBRARY_VISIBILITY M68kTargetInfo : public TargetInfo {
+  static const char *const GCCRegNames[];
+
+  enum CPUKind {
+    CK_Unknown,
+    CK_68000,
+    CK_68010,
+    CK_68020,
+    CK_68030,
+    CK_68040
+  } CPU = CK_Unknown;
+
+public:
+  M68kTargetInfo(const llvm::Triple &Triple, const TargetOptions &);
+
+  void getTargetDefines(const LangOptions &Opts,
+                        MacroBuilder &Builder) const override;
+  ArrayRef<Builtin::Info> getTargetBuiltins() const override;
+  bool hasFeature(StringRef Feature) const override;
+  ArrayRef<const char *> getGCCRegNames() const override;
+  ArrayRef<TargetInfo::GCCRegAlias> getGCCRegAliases() const override;
+  bool validateAsmConstraint(const char *&Name,
+                             TargetInfo::ConstraintInfo &info) const override;
+  const char *getClobbers() const override;
+  BuiltinVaListKind getBuiltinVaListKind() const override;
+  bool setCPU(const std::string &Name) override;
+};
+
+} // namespace targets
+} // namespace clang
+
+#endif /* end of include guard: M680X0_H_LTNCIPAD */

clang/lib/Basic/Targets/M68k.cpp

+//===--- M68k.cpp - Implement M68k targets feature support-------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file implements M68k TargetInfo objects.
+//
+//===----------------------------------------------------------------------===//
+
+#include "M68k.h"
+#include "clang/Basic/Builtins.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/TargetBuiltins.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSwitch.h"
+#include "llvm/Support/TargetParser.h"
+#include <cstring>
+
+namespace clang {
+namespace targets {
+
+M68kTargetInfo::M68kTargetInfo(const llvm::Triple &Triple,
+                               const TargetOptions &)
+    : TargetInfo(Triple) {
+
+  std::string Layout = "";
+
+  // M68k is Big Endian
+  Layout += "E";
+
+  // FIXME how to wire it with the used object format?
+  Layout += "-m:e";
+
+  // M68k pointers are always 32 bit wide even for 16 bit cpus
+  Layout += "-p:32:32";
+
+  // M68k integer data types
+  Layout += "-i8:8:8-i16:16:16-i32:32:32";
+
+  // FIXME no floats at the moment
+
+  // The registers can hold 8, 16, 32 bits
+  Layout += "-n8:16:32";
+
+  // Aggregates are 32 bit aligned and stack is 16 bit aligned
+  Layout += "-a:0:32-S16";
+
+  resetDataLayout(Layout);
+
+  SizeType = UnsignedInt;
+  PtrDiffType = SignedInt;
+  IntPtrType = SignedInt;
+}
+
+bool M68kTargetInfo::setCPU(const std::string &Name) {
+  StringRef N = Name;
+  CPU = llvm::StringSwitch<CPUKind>(N)
+            .Case("generic", CK_68000)
+            .Case("M68000", CK_68000)
+            .Case("M68010", CK_68010)
+            .Case("M68020", CK_68020)
+            .Case("M68030", CK_68030)
+            .Case("M68040", CK_68040)
+            .Default(CK_Unknown);
+  return CPU != CK_Unknown;
+}
+
+void M68kTargetInfo::getTargetDefines(const LangOptions &Opts,
+                                      MacroBuilder &Builder) const {
+  using llvm::Twine;
+
+  Builder.defineMacro("M68k");
+  Builder.defineMacro("__M68k__");
+  Builder.defineMacro("__M68K__");
+  Builder.defineMacro("__m68k__");
+
+  Builder.defineMacro("mc68000");
+  Builder.defineMacro("__mc68000");
+  Builder.defineMacro("__mc68000__");
+
+  // For sub-architecture
+  switch (CPU) {
+  case CK_68010:
+    Builder.defineMacro("mc68010");
+    Builder.defineMacro("__mc68010");
+    Builder.defineMacro("__mc68010__");
+    break;
+  case CK_68020:
+    Builder.defineMacro("mc68020");
+    Builder.defineMacro("__mc68020");
+    Builder.defineMacro("__mc68020__");
+    break;
+  case CK_68030:
+    Builder.defineMacro("mc68030");
+    Builder.defineMacro("__mc68030");
+    Builder.defineMacro("__mc68030__");
+    break;
+  case CK_68040:
+    Builder.defineMacro("mc68040");
+    Builder.defineMacro("__mc68040");
+    Builder.defineMacro("__mc68040__");
+    break;
+  default:
+    break;
+  }
+}
+
+ArrayRef<Builtin::Info> M68kTargetInfo::getTargetBuiltins() const {
+  // FIXME: Implement.
+  return None;
+}
+
+bool M68kTargetInfo::hasFeature(StringRef Feature) const {
+  // FIXME elaborate moar
+  return Feature == "M68000";
+}
+
+const char *const M68kTargetInfo::GCCRegNames[] = {
+    "d0", "d1", "d2", "d3", "d4", "d5", "d6", "d7",
+    "a0", "a1", "a2", "a3", "a4", "a5", "a6", "a7",
+    "pc"};
+
+ArrayRef<const char *> M68kTargetInfo::getGCCRegNames() const {
+  return llvm::makeArrayRef(GCCRegNames);
+}
+
+ArrayRef<TargetInfo::GCCRegAlias> M68kTargetInfo::getGCCRegAliases() const {
+  // No aliases.
+  return None;
+}
+
+bool M68kTargetInfo::validateAsmConstraint(
+    const char *&Name, TargetInfo::ConstraintInfo &info) const {
+  // FIXME: implement
+  switch (*Name) {
+  case 'K': // the constant 1
+  case 'L': // constant -1^20 .. 1^19
+  case 'M': // constant 1-4:
+    return true;
+  }
+  // No targets constraints for now.
+  return false;
+}
+
+const char *M68kTargetInfo::getClobbers() const {
+  // FIXME: Is this really right?
+  return "";
+}
+
+M68kTargetInfo::BuiltinVaListKind M68kTargetInfo::getBuiltinVaListKind() const {
+  // FIXME: implement
+  return TargetInfo::CharPtrBuiltinVaList;
+}
+
+} // namespace targets
+} // namespace clang

clang/lib/CodeGen/TargetInfo.cpp

MIPSTargetCodeGenInfo::initDwarfEHRegSizeTable(CodeGen::CodeGenFunction &CGF,
   // 112-143 are the coprocessor 2 registers, $c2r0 - $c2r31.
   // 144-175 are the coprocessor 3 registers, $c3r0 - $c3r31.
   // 176-181 are the DSP accumulator registers.
   AssignToArrayRange(CGF.Builder, Address, Four8, 80, 181);
   return false;
 }
 
 //===----------------------------------------------------------------------===//
+// M68k ABI Implementation
+//===----------------------------------------------------------------------===//
+
+namespace {
+
+class M68kTargetCodeGenInfo : public TargetCodeGenInfo {
+public:
+  M68kTargetCodeGenInfo(CodeGenTypes &CGT)
+      : TargetCodeGenInfo(std::make_unique<DefaultABIInfo>(CGT)) {}
+  void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
+                           CodeGen::CodeGenModule &M) const override;
+};
+
+} // namespace
+
+void M68kTargetCodeGenInfo::setTargetAttributes(
+    const Decl *D, llvm::GlobalValue *GV, CodeGen::CodeGenModule &M) const {
+  if (const auto *FD = dyn_cast_or_null<FunctionDecl>(D)) {
+    if (const auto *attr = FD->getAttr<M68kInterruptAttr>()) {
+      // Handle 'interrupt' attribute:
+      llvm::Function *F = cast<llvm::Function>(GV);
+
+      // Step 1: Set ISR calling convention.
+      F->setCallingConv(llvm::CallingConv::M68k_INTR);
+
+      // Step 2: Add attributes goodness.
+      F->addFnAttr(llvm::Attribute::NoInline);
+
+      // Step 3: Emit ISR vector alias.
+      unsigned Num = attr->getNumber() / 2;
+      llvm::GlobalAlias::create(llvm::Function::ExternalLinkage,
+                                "__isr_" + Twine(Num), F);
+    }
+  }
+}
+
+//===----------------------------------------------------------------------===//
 // AVR ABI Implementation.
 //===----------------------------------------------------------------------===//
 
 namespace {
 class AVRTargetCodeGenInfo : public TargetCodeGenInfo {
 public:
   AVRTargetCodeGenInfo(CodeGenTypes &CGT)
       : TargetCodeGenInfo(std::make_unique<DefaultABIInfo>(CGT)) {}
const TargetCodeGenInfo &CodeGenModule::getTargetCodeGenInfo() {
 
   const llvm::Triple &Triple = getTarget().getTriple();
   switch (Triple.getArch()) {
   default:
     return SetCGInfo(new DefaultTargetCodeGenInfo(Types));
 
   case llvm::Triple::le32:
     return SetCGInfo(new PNaClTargetCodeGenInfo(Types));
+  case llvm::Triple::m68k:
+    return SetCGInfo(new M68kTargetCodeGenInfo(Types));
   case llvm::Triple::mips:
   case llvm::Triple::mipsel:
     if (Triple.getOS() == llvm::Triple::NaCl)
       return SetCGInfo(new PNaClTargetCodeGenInfo(Types));
     return SetCGInfo(new MIPSTargetCodeGenInfo(Types, true));
 
   case llvm::Triple::mips64:
   case llvm::Triple::mips64el:

clang/lib/Sema/SemaDeclAttr.cpp

if (!MipsInterruptAttr::ConvertStrToInterruptType(Str, Kind)) {
     S.Diag(AL.getLoc(), diag::warn_attribute_type_not_supported)
         << AL << "'" + std::string(Str) + "'";
     return;
   }
 
   D->addAttr(::new (S.Context) MipsInterruptAttr(S.Context, AL, Kind));
 }
 
+static void handleM68kInterruptAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+  if (!checkAttributeNumArgs(S, AL, 1))
+    return;
+
+  if (!AL.isArgExpr(0)) {
+    S.Diag(AL.getLoc(), diag::err_attribute_argument_type)
+        << AL << AANT_ArgumentIntegerConstant;
+    return;
+  }
+
+  // FIXME: Check for decl - it should be void ()(void).
+
+  Expr *NumParamsExpr = static_cast<Expr *>(AL.getArgAsExpr(0));
+  auto MaybeNumParams = NumParamsExpr->getIntegerConstantExpr(S.Context);
+  if (!MaybeNumParams) {
+    S.Diag(AL.getLoc(), diag::err_attribute_argument_type)
+        << AL << AANT_ArgumentIntegerConstant
+        << NumParamsExpr->getSourceRange();
+    return;
+  }
+
+  unsigned Num = MaybeNumParams->getLimitedValue(255);
+  if ((Num & 1) || Num > 30) {
+    S.Diag(AL.getLoc(), diag::err_attribute_argument_out_of_bounds)
+        << AL << (int)MaybeNumParams->getSExtValue()
+        << NumParamsExpr->getSourceRange();
+    return;
+  }
+
+  D->addAttr(::new (S.Context) M68kInterruptAttr(S.Context, AL, Num));
+  D->addAttr(UsedAttr::CreateImplicit(S.Context));
+}
+
 static void handleAnyX86InterruptAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   // Semantic checks for a function with the 'interrupt' attribute.
   // a) Must be a function.
   // b) Must have the 'void' return type.
   // c) Must take 1 or 2 arguments.
   // d) The 1st argument must be a pointer.
   // e) The 2nd argument (if any) must be an unsigned integer.
   if (!isFunctionOrMethod(D) || !hasFunctionProto(D) || isInstanceMethod(D) ||
static void handleInterruptAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   switch (S.Context.getTargetInfo().getTriple().getArch()) {
   case llvm::Triple::msp430:
     handleMSP430InterruptAttr(S, D, AL);
     break;
   case llvm::Triple::mipsel:
   case llvm::Triple::mips:
     handleMipsInterruptAttr(S, D, AL);
     break;
+  case llvm::Triple::m68k:
+    handleM68kInterruptAttr(S, D, AL);
+    break;
   case llvm::Triple::x86:
   case llvm::Triple::x86_64:
     handleAnyX86InterruptAttr(S, D, AL);
     break;
   case llvm::Triple::avr:
     handleAVRInterruptAttr(S, D, AL);
     break;
   case llvm::Triple::riscv32: