Skip to content

Conversation

@student433
Copy link

@student433 student433 commented Feb 4, 2025

Fixes #114979, adding support for the cl2000 driver and c2000 architecture to frontend to get clangd working as discussed in https://discourse.llvm.org/t/ti-c2000-target-not-supported-in-clangd-lsp/83015

@github-actions
Copy link

github-actions bot commented Feb 4, 2025

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Feb 4, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 4, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: None (student433)

Changes

Fixes #114979, adding support for the cl2000 compiler to the clang frontend to get clangd working as discussed in https://discourse.llvm.org/t/ti-c2000-target-not-supported-in-clangd-lsp/83015


Patch is 39.21 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/125663.diff

15 Files Affected:

  • (modified) clang/include/clang/Driver/Driver.h (+5-1)
  • (modified) clang/include/clang/Driver/Options.h (+1)
  • (modified) clang/include/clang/Driver/Options.td (+64-10)
  • (modified) clang/lib/Basic/CMakeLists.txt (+1)
  • (modified) clang/lib/Basic/Targets.cpp (+4)
  • (added) clang/lib/Basic/Targets/C2000.cpp (+356)
  • (added) clang/lib/Basic/Targets/C2000.h (+100)
  • (modified) clang/lib/Driver/CMakeLists.txt (+1)
  • (modified) clang/lib/Driver/Driver.cpp (+13)
  • (modified) clang/lib/Driver/ToolChain.cpp (+1)
  • (added) clang/lib/Driver/ToolChains/Arch/C2000.cpp (+85)
  • (added) clang/lib/Driver/ToolChains/Arch/C2000.h (+22)
  • (modified) clang/lib/Driver/ToolChains/CommonArgs.cpp (+5)
  • (modified) llvm/include/llvm/TargetParser/Triple.h (+1)
  • (modified) llvm/lib/TargetParser/Triple.cpp (+7)
diff --git a/clang/include/clang/Driver/Driver.h b/clang/include/clang/Driver/Driver.h
index f4a52cc529b79c..55da823598f9b1 100644
--- a/clang/include/clang/Driver/Driver.h
+++ b/clang/include/clang/Driver/Driver.h
@@ -107,7 +107,8 @@ class Driver {
     CPPMode,
     CLMode,
     FlangMode,
-    DXCMode
+    DXCMode,
+    C2000Mode
   } Mode;
 
   enum SaveTempsMode {
@@ -253,6 +254,9 @@ class Driver {
   /// Whether the driver should follow dxc.exe like behavior.
   bool IsDXCMode() const { return Mode == DXCMode; }
 
+  // Whether the driver should follow cl2000.exe like behaviour.
+  bool IsC2000Mode() const { return Mode == C2000Mode; }
+
   /// Only print tool bindings, don't build any jobs.
   LLVM_PREFERRED_TYPE(bool)
   unsigned CCCPrintBindings : 1;
diff --git a/clang/include/clang/Driver/Options.h b/clang/include/clang/Driver/Options.h
index 0797410e9940e2..1613810a5bf741 100644
--- a/clang/include/clang/Driver/Options.h
+++ b/clang/include/clang/Driver/Options.h
@@ -39,6 +39,7 @@ enum ClangVisibility {
   FlangOption = (1 << 4),
   FC1Option = (1 << 5),
   DXCOption = (1 << 6),
+  CL2000Option = (1 << 7)
 };
 
 enum ID {
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index d38dd2b4e3cf09..44ce3c3413b09b 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -97,6 +97,10 @@ def FC1Option : OptionVisibility;
 // are made available when the driver is running in DXC compatibility mode.
 def DXCOption : OptionVisibility;
 
+// CL2000Option - This is a cl2000.exe compatibility option. Options with this flag
+// are made available when the driver is running in cl2000 compatibility mode.
+def CL2000Option : OptionVisibility;
+
 /////////
 // Docs
 
@@ -201,6 +205,9 @@ def hlsl_Group : OptionGroup<"<HLSL group>">, Group<f_Group>,
                    DocName<"HLSL options">,
                    Visibility<[ClangOption]>;
 
+def cl2000_group : OptionGroup<"<cl2000 group>">,
+                    Visibility<[CL2000Option]>;
+
 // Feature groups - these take command line options that correspond directly to
 // target specific features and can be translated directly from command line
 // options.
@@ -672,7 +679,7 @@ class InternalDriverOpt : Group<internal_driver_Group>,
   Flags<[NoXarchOption, HelpHidden]>;
 def driver_mode : Joined<["--"], "driver-mode=">, Group<internal_driver_Group>,
   Flags<[NoXarchOption, HelpHidden]>,
-  Visibility<[ClangOption, FlangOption, CLOption, DXCOption]>,
+  Visibility<[ClangOption, FlangOption, CLOption, DXCOption, CL2000Option]>,
   HelpText<"Set the driver mode to either 'gcc', 'g++', 'cpp', 'cl' or 'flang'">;
 def rsp_quoting : Joined<["--"], "rsp-quoting=">, Group<internal_driver_Group>,
   Flags<[NoXarchOption, HelpHidden]>,
@@ -843,7 +850,7 @@ def C : Flag<["-"], "C">, Visibility<[ClangOption, CC1Option]>,
     HelpText<"Include comments in preprocessed output">,
     MarshallingInfoFlag<PreprocessorOutputOpts<"ShowComments">>;
 def D : JoinedOrSeparate<["-"], "D">, Group<Preprocessor_Group>,
-    Visibility<[ClangOption, CC1Option, FlangOption, FC1Option, DXCOption]>,
+    Visibility<[ClangOption, CC1Option, FlangOption, FC1Option, DXCOption, CL2000Option]>,
     MetaVarName<"<macro>=<value>">,
     HelpText<"Define <macro> to <value> (or 1 if <value> omitted)">;
 def E : Flag<["-"], "E">, Flags<[NoXarchOption]>,
@@ -929,7 +936,7 @@ def ObjCXX : Flag<["-"], "ObjC++">, Flags<[NoXarchOption]>,
 def ObjC : Flag<["-"], "ObjC">, Flags<[NoXarchOption]>,
   HelpText<"Treat source input files as Objective-C inputs">;
 def O : Joined<["-"], "O">, Group<O_Group>,
-  Visibility<[ClangOption, CC1Option, FC1Option, FlangOption]>;
+  Visibility<[ClangOption, CC1Option, FC1Option, FlangOption, CL2000Option]>;
 def O_flag : Flag<["-"], "O">, Visibility<[ClangOption, CC1Option, FC1Option]>,
   Alias<O>, AliasArgs<["1"]>;
 def Ofast : Joined<["-"], "Ofast">, Group<O_Group>,
@@ -1035,10 +1042,10 @@ def Xassembler : Separate<["-"], "Xassembler">,
   Group<CompileOnly_Group>;
 def Xclang : Separate<["-"], "Xclang">,
   HelpText<"Pass <arg> to clang -cc1">, MetaVarName<"<arg>">,
-  Flags<[NoXarchOption]>, Visibility<[ClangOption, CLOption, DXCOption]>,
+  Flags<[NoXarchOption]>, Visibility<[ClangOption, CLOption, DXCOption, CL2000Option]>,
   Group<CompileOnly_Group>;
 def : Joined<["-"], "Xclang=">, Group<CompileOnly_Group>,
-  Flags<[NoXarchOption]>, Visibility<[ClangOption, CLOption, DXCOption]>,
+  Flags<[NoXarchOption]>, Visibility<[ClangOption, CLOption, DXCOption, CL2000Option]>,
   Alias<Xclang>,
   HelpText<"Alias for -Xclang">, MetaVarName<"<arg>">;
 def Xcuda_fatbinary : Separate<["-"], "Xcuda-fatbinary">,
@@ -4023,7 +4030,7 @@ def fdriver_only : Flag<["-"], "fdriver-only">, Flags<[NoXarchOption]>,
   Group<Action_Group>, HelpText<"Only run the driver.">;
 def fsyntax_only : Flag<["-"], "fsyntax-only">,
   Flags<[NoXarchOption]>,
-  Visibility<[ClangOption, CLOption, DXCOption, CC1Option, FC1Option, FlangOption]>,
+  Visibility<[ClangOption, CLOption, DXCOption, CC1Option, FC1Option, FlangOption, CL2000Option]>,
   Group<Action_Group>,
   HelpText<"Run the preprocessor, parser and semantic analysis stages">;
 def ftabstop_EQ : Joined<["-"], "ftabstop=">, Group<f_Group>;
@@ -4470,7 +4477,7 @@ defm emit_compact_unwind_non_canonical : BoolFOption<"emit-compact-unwind-non-ca
           "Try emitting Compact-Unwind for non-canonical entries. Maybe overridden by other constraints">,
   NegFlag<SetFalse>>;
 def g_Flag : Flag<["-"], "g">, Group<g_Group>,
-    Visibility<[ClangOption, CLOption, DXCOption, FlangOption]>,
+    Visibility<[ClangOption, CLOption, DXCOption, FlangOption, CL2000Option]>,
     HelpText<"Generate source-level debug information">;
 def gline_tables_only : Flag<["-"], "gline-tables-only">, Group<gN_Group>,
   Visibility<[ClangOption, CLOption, DXCOption, FlangOption]>,
@@ -5817,11 +5824,11 @@ def rdynamic : Flag<["-"], "rdynamic">, Group<Link_Group>,
   Visibility<[ClangOption, FlangOption]>;
 def resource_dir : Separate<["-"], "resource-dir">,
   Flags<[NoXarchOption, HelpHidden]>,
-  Visibility<[ClangOption, CC1Option, CLOption, DXCOption, FlangOption, FC1Option]>,
+  Visibility<[ClangOption, CC1Option, CLOption, DXCOption, FlangOption, FC1Option, CL2000Option]>,
   HelpText<"The directory which holds the compiler resource files">,
   MarshallingInfoString<HeaderSearchOpts<"ResourceDir">>;
 def resource_dir_EQ : Joined<["-"], "resource-dir=">, Flags<[NoXarchOption]>,
-  Visibility<[ClangOption, CLOption, DXCOption, FlangOption]>,
+  Visibility<[ClangOption, CLOption, DXCOption, FlangOption, CL2000Option]>,
   Alias<resource_dir>;
 def rpath : Separate<["-"], "rpath">, Flags<[LinkerInput]>, Group<Link_Group>,
   Visibility<[ClangOption, FlangOption]>;
@@ -6020,7 +6027,7 @@ def w : Flag<["-"], "w">, HelpText<"Suppress all warnings">,
   MarshallingInfoFlag<DiagnosticOpts<"IgnoreWarnings">>;
 def x : JoinedOrSeparate<["-"], "x">,
 Flags<[NoXarchOption]>,
-  Visibility<[ClangOption, CC1Option, FlangOption, FC1Option, CLOption]>,
+  Visibility<[ClangOption, CC1Option, FlangOption, FC1Option, CLOption, CL2000Option]>,
   HelpText<"Treat subsequent input files as having type <language>">,
   MetaVarName<"<language>">;
 def y : Joined<["-"], "y">;
@@ -9111,3 +9118,50 @@ def wasm_opt : Flag<["--"], "wasm-opt">,
   Group<m_Group>,
   HelpText<"Enable the wasm-opt optimizer (default)">,
   MarshallingInfoNegativeFlag<LangOpts<"NoWasmOpt">>;
+
+
+
+//===----------------------------------------------------------------------===//
+// cl2000 Options
+//===----------------------------------------------------------------------===//
+
+
+
+
+def cl2000_include_path : Joined<["--"], "include_path=">, Group<cl2000_group>,
+                    HelpText<"specify include search paths for cl2000 driver mode">, Alias<isystem>;
+
+def eabi :        Joined<["--"], "abi=">, Group<cl2000_group>,
+                  HelpText<"abi">;
+def strict_ansi : Joined<["--"], "strict_ansi">, Group<cl2000_group>,
+                  HelpText<"strict ANSI/ISO Mode">;
+def fp_mode :     Joined<["--"], "fp_mode=">, Group<cl2000_group>,
+                  HelpText<"fp mode">;
+def cla_support : Joined<["--"], "cla_support=">, Group<cl2000_group>,
+                  HelpText<"cla Support">;
+def float_support :   Joined<["--"], "float_support=">, Group<cl2000_group>,
+                      HelpText<"Float Support">;
+def idiv_support  :   Joined<["--"], "idiv_support=">, Group<cl2000_group>,
+                      HelpText<"idiv Support">;
+def tmu_support   :   Joined<["--"], "tmu_support=">, Group<cl2000_group>,
+                      HelpText<"tmu Support">;
+def vcu_support   :   Joined<["--"], "vcu_support=">, Group<cl2000_group>,
+                      HelpText<"vcu Support">;
+def opt_level   :   Joined<["--"], "opt_level=">, Group<cl2000_group>,
+                      HelpText<"opt level">, Alias<O>;
+def silicon_version     :   Joined<["-"], "v28">, Group<cl2000_group>,
+                    HelpText<"silicon version">;
+def large_model    :   Joined<["-"], "ml">, Group<cl2000_group>,
+                    HelpText<"large model">;
+def unified_memory     :   Joined<["-"], "mt">, Group<cl2000_group>,
+                    HelpText<"unified memory">;
+def output_file     :   Joined<["--"], "output_file=">, Group<cl2000_group>,
+                      HelpText<"output file">, Alias<o>;
+def source_file     :   Joined<["--"], "">, Group<cl2000_group>,
+                      HelpText<"source file">;
+def symdebug_dwarf     :   Joined<["--"], "symdebug:dwarf">, Group<cl2000_group>,
+                      HelpText<"Alias for -g">, Alias<g_Flag>;
+def relaxed_ansi     :   Joined<["--"], "relaxed_ansi">, Group<cl2000_group>,
+                      HelpText<"relaxed ansi mode">;
+def compile_only   :   Joined<["--"], "compile_only">, Group<cl2000_group>,
+                      HelpText<"compile only">, Alias<c>;
diff --git a/clang/lib/Basic/CMakeLists.txt b/clang/lib/Basic/CMakeLists.txt
index 331dfbb3f4b67e..bc71f93fe101a2 100644
--- a/clang/lib/Basic/CMakeLists.txt
+++ b/clang/lib/Basic/CMakeLists.txt
@@ -99,6 +99,7 @@ add_clang_library(clangBasic
   Targets/ARM.cpp
   Targets/AVR.cpp
   Targets/BPF.cpp
+  Targets/C2000.cpp
   Targets/CSKY.cpp
   Targets/DirectX.cpp
   Targets/Hexagon.cpp
diff --git a/clang/lib/Basic/Targets.cpp b/clang/lib/Basic/Targets.cpp
index 281aebdb1c35d3..9f5045a73b2d83 100644
--- a/clang/lib/Basic/Targets.cpp
+++ b/clang/lib/Basic/Targets.cpp
@@ -19,6 +19,7 @@
 #include "Targets/ARM.h"
 #include "Targets/AVR.h"
 #include "Targets/BPF.h"
+#include "Targets/C2000.h"
 #include "Targets/CSKY.h"
 #include "Targets/DirectX.h"
 #include "Targets/Hexagon.h"
@@ -272,6 +273,9 @@ std::unique_ptr<TargetInfo> AllocateTarget(const llvm::Triple &Triple,
   case llvm::Triple::msp430:
     return std::make_unique<MSP430TargetInfo>(Triple, Opts);
 
+  case llvm::Triple::c2000:
+    return std::make_unique<C2000TargetInfo>(Triple, Opts);
+
   case llvm::Triple::mips:
     switch (os) {
     case llvm::Triple::Linux:
diff --git a/clang/lib/Basic/Targets/C2000.cpp b/clang/lib/Basic/Targets/C2000.cpp
new file mode 100644
index 00000000000000..5fe53377f10878
--- /dev/null
+++ b/clang/lib/Basic/Targets/C2000.cpp
@@ -0,0 +1,356 @@
+#include "C2000.h"
+#include "Targets.h"
+#include "clang/Basic/Builtins.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/MacroBuilder.h"
+#include "clang/Basic/TargetBuiltins.h"
+
+using namespace clang;
+using namespace clang::targets;
+
+const char *const C2000TargetInfo::GCCRegNames[] = {
+    "ACC", "XAR0", "XAR1", "XAR2", "XAR3", "XAR4", "XAR5", "XAR6", "XAR7"};
+
+ArrayRef<const char *> C2000TargetInfo::getGCCRegNames() const {
+  return llvm::ArrayRef(GCCRegNames);
+}
+
+bool C2000TargetInfo::handleTargetFeatures(std::vector<std::string> &Features,
+                                           DiagnosticsEngine &Diags) {
+
+  for (const auto &Feature : Features) {
+    if (Feature == "+eabi") {
+      eabi = true;
+      continue;
+    }
+    if (Feature == "+strict_ansi") {
+      strict = true;
+      continue;
+    }
+    if (Feature == "+cla_support") {
+      cla_support = true;
+    }
+    if (Feature == "+cla0") {
+      cla0 = true;
+      continue;
+    }
+    if (Feature == "+cla1") {
+      cla1 = true;
+      continue;
+    }
+    if (Feature == "+cla2") {
+      cla2 = true;
+      continue;
+    }
+    if (Feature == "+relaxed") {
+      relaxed = true;
+      continue;
+    }
+    if (Feature == "+fpu64") {
+      fpu64 = true;
+      continue;
+    }
+    if (Feature == "+fpu32") {
+      fpu32 = true;
+      continue;
+    }
+    if (Feature == "+tmu_support") {
+      tmu_support = true;
+    }
+    if (Feature == "+tmu1") {
+      tmu1 = true;
+      continue;
+    }
+    if (Feature == "+idiv0") {
+      idiv0 = true;
+      continue;
+    }
+    if (Feature == "+vcu_support") {
+      vcu_support = true;
+    }
+    if (Feature == "+vcu2") {
+      vcu2 = true;
+      continue;
+    }
+    if (Feature == "+vcrc") {
+      vcrc = true;
+      continue;
+    }
+    if (Feature == "+opt_level") {
+      opt = true;
+      continue;
+    }
+  }
+  return true;
+}
+
+bool C2000TargetInfo::hasFeature(StringRef Feature) const {
+  return llvm::StringSwitch<bool>(Feature)
+      .Case("eabi", eabi)
+      .Case("strict_ansi", strict)
+      .Case("cla-support", cla_support)
+      .Case("cla0", cla0)
+      .Case("cla1", cla1)
+      .Case("cla2", cla2)
+      .Case("relaxed", relaxed)
+      .Case("fpu64", fpu64)
+      .Case("fpu32", fpu32)
+      .Case("tmu-support", tmu_support)
+      .Case("tmu1", tmu1)
+      .Case("vcu-support", vcu_support)
+      .Case("vcu2", vcu2)
+      .Case("vcrc", vcrc)
+      .Case("opt-level", opt)
+      .Default(false);
+}
+
+void C2000TargetInfo::getTargetDefines(const LangOptions &Opts,
+                                       MacroBuilder &Builder) const {
+  Builder.undefineMacro("__CHAR_BIT__"); // FIXME: Implement 16-bit char
+  Builder.defineMacro("__CHAR_BIT__", "16");
+  Builder.defineMacro("__TMS320C2000__");
+  Builder.defineMacro("_TMS320C2000");
+  Builder.defineMacro("__TMS320C28XX__");
+  Builder.defineMacro("_TMS320C28XX");
+  Builder.defineMacro("__TMS320C28X__");
+  Builder.defineMacro("_TMS320C28X");
+  Builder.defineMacro("__TI_STRICT_FP_MODE__");
+  Builder.defineMacro("__COMPILER_VERSION__");
+  Builder.defineMacro("__TI_COMPILER_VERSION__");
+  Builder.defineMacro("__TI_COMPILER_VERSION__QUAL_ID");
+  Builder.defineMacro("__TI_COMPILER_VERSION__QUAL__", "QUAL_LETTER");
+  Builder.defineMacro("__little_endian__");
+  Builder.defineMacro("__PTRDIFF_T_TYPE__", "signed long");
+  Builder.defineMacro("__SIZE_T_TYPE__", "unsigned long");
+  Builder.defineMacro("__WCHAR_T_TYPE__", "long unsigned");
+  Builder.defineMacro("__TI_WCHAR_T_BITS", "16");
+  Builder.defineMacro("__TI_C99_COMPLEX_ENABLED");
+  Builder.defineMacro("__TI_GNU_ATTRIBUTE_SUPPORT__");
+  Builder.defineMacro("__LARGE_MODEL__");
+  Builder.defineMacro("__signed_chars__");
+  Builder.defineMacro("__OPTIMIZE_FOR_SPACE");
+
+  if (hasFeature("eabi"))
+    Builder.defineMacro("__TI_EABI__");
+  if (hasFeature("strict_ansi"))
+    Builder.defineMacro("__TI_STRICT_ANSI_MODE__");
+  if (hasFeature("cla-support"))
+    Builder.defineMacro("__TMS320C28XX_CLA__");
+
+  if (hasFeature("cla0"))
+    Builder.defineMacro("__TMS320C28XX_CLA0__");
+  else if (hasFeature("cla1"))
+    Builder.defineMacro("__TMS320C28XX_CLA1__");
+  else if (hasFeature("cla2"))
+    Builder.defineMacro("__TMS320C28XX_CLA2__");
+
+  if (hasFeature("fpu64")) {
+    Builder.defineMacro("__TMS320C28XX_FPU64__");
+    Builder.defineMacro("__TMS320C28XX_FPU32__");
+  } else if (hasFeature("fpu32"))
+    Builder.defineMacro("__TMS320C28XX_FPU32__");
+  if (hasFeature("idiv0"))
+    Builder.defineMacro("__TMS320C28XX_IDIV__");
+  if (hasFeature("tmu1"))
+    Builder.defineMacro("__TMS320C28XX_TMU1__");
+  if (hasFeature("tmu-support")) {
+    Builder.defineMacro("__TMS320C28XX_TMU0__");
+    Builder.defineMacro("__TMS320C28XX_TMU__");
+  }
+  if (hasFeature("vcu-support"))
+    Builder.defineMacro("__TMS320C28XX_VCU0__");
+  if (hasFeature("vcu2"))
+    Builder.defineMacro("__TMS320C28XX_VCU2__");
+  else if (hasFeature("vcrc"))
+    Builder.defineMacro("__TMS320C28XX_VCRC__");
+  if (hasFeature("opt-level"))
+    Builder.defineMacro("_INLINE");
+  if (hasFeature("relaxed"))
+    Builder.undefineMacro("__TI_STRICT_FP_MODE__");
+
+  Builder.defineMacro("__cregister", "");
+  Builder.defineMacro("interrupt", "");
+  Builder.defineMacro("__interrupt", "");
+
+  // Assembly Instrinsics
+
+  Builder.append("int __abs16_sat( int src );");
+  Builder.append("void __add( int *m, int b );");
+  Builder.append("long __addcu( long src1, unsigned int src2 );");
+  Builder.append("void __addl( long *m, long b );");
+  Builder.append("void __and(int *m, int b);");
+  Builder.append("int *__byte_func( int *array, unsigned int byte_index );");
+  Builder.defineMacro("__byte(array, byte_index)",
+                      "*__byte_func(array, byte_index)");
+  Builder.append("unsigned long *__byte_peripheral_32_func(unsigned long *x);");
+  Builder.defineMacro("__byte_peripheral_32(x)",
+                      "*__byte_peripheral_32_func(x)");
+  Builder.append("void __dec( int *m );");
+
+  // dmac needs macro magic
+  Builder.append("unsigned int __disable_interrupts( );");
+  Builder.append("void __eallow( void );");
+  Builder.append("void __edis( void );");
+  Builder.append("unsigned int __enable_interrupts( );");
+  Builder.append("int __flip16(int src);");
+  Builder.append("long __flip32(long src);");
+  Builder.append("long long __flip64(long long src);");
+  Builder.append("void __inc( int *m );");
+  Builder.append("long __IQ( long double A , int N );");
+  Builder.append("long __IQmpy( long A, long B , int N );");
+  Builder.append("long __IQsat( long A, long max, long min );");
+  Builder.append("long __IQxmpy(long A , long B, int N);");
+  Builder.append("long long __llmax(long long dst, long long src);");
+  Builder.append("long long __llmin(long long dst, long long src);");
+  Builder.append("long __lmax(long dst, long src);");
+  Builder.append("long __lmin(long dst, long src);");
+  Builder.append("int __max(int dst, int src);");
+  Builder.append("int __min(int dst, int src);");
+  Builder.append("int __mov_byte( int *src, unsigned int n );");
+  Builder.append("long __mpy( int src1, int src2 );");
+  Builder.append("long __mpyb( int src1, unsigned int src2 );");
+  Builder.append("long __mpy_mov_t( int src1, int src2, int * dst2 );");
+  Builder.append("unsigned long __mpyu(unsigned int src2, unsigned int srt2);");
+  Builder.append("long __mpyxu( int src1, unsigned int src2 );");
+  Builder.append("long __norm32(long src, int * shift );");
+  Builder.append("long long __norm64(long long src, int * shift );");
+  Builder.append("void __or(int *m, int b);");
+  Builder.append("long __qmpy32( long src32a, long src32b, int q );");
+  Builder.append("long __qmpy32by16(long src32, int src16, int q);");
+  Builder.append("void __restore_interrupts(unsigned int val );");
+  Builder.append("long __rol( long src );");
+  Builder.append("long __ror( long src );");
+  Builder.append("void * __rpt_mov_imm(void * dst , int src ,int count );");
+  Builder.append("int __rpt_norm_inc( long src, int dst, int count );");
+  Builder.append("int __rpt_norm_dec(long src, int dst, int count);");
+  Builder.append("long __rpt_rol(long src, int count);");
+  Builder.append("long __rpt_ror(long src, int count);");
+  Builder.append("long __rpt_subcu(long dst, int src, int count);");
+  Builder.append("unsigned long __rpt_subcul(unsigned long num, unsigned long "
+                 "den, unsigned long remainder, int count);");
+  Builder.append("long __sat( long src );");
+  Builder.append("long __sat32( long src, long limit );");
+  Builder.append("long __sathigh16(long src, int limit);");
+  Builder.append("long __satlow16( long src );");
+  Builder.append("long __sbbu( long src1 , unsigned int src2 );");
+  Builder.append("void __sub( int * m, int b );");
+  Builder.append("long __subcu( long src1, int src2 );");
+  Builder.append("unsigned long __subcul(unsigned long num, unsigned long den, "
+                 "unsigned long remainder);");
+  Builder.append("void __subl( long * m, long b )...
[truncated]

@github-actions
Copy link

github-actions bot commented Feb 4, 2025

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 8368018f2097e330a6b6ec0a9372487df020c511 5906c0f4090e2e9b790401c34ba70eedcbb7f8ae --extensions h,cpp -- clang/lib/Basic/Targets/C2000.cpp clang/lib/Basic/Targets/C2000.h clang/lib/Driver/ToolChains/Arch/C2000.cpp clang/lib/Driver/ToolChains/Arch/C2000.h clang/include/clang/Driver/Driver.h clang/include/clang/Driver/Options.h clang/lib/Basic/Targets.cpp clang/lib/Driver/Driver.cpp clang/lib/Driver/ToolChain.cpp clang/lib/Driver/ToolChains/CommonArgs.cpp llvm/include/llvm/TargetParser/Triple.h llvm/lib/TargetParser/Triple.cpp
View the diff from clang-format here.
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 9ccccda2ed..16fc7bb461 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -301,7 +301,7 @@ void Driver::setDriverMode(StringRef Value) {
                    .Case("cl", CLMode)
                    .Case("flang", FlangMode)
                    .Case("dxc", DXCMode)
-				           .Case("cl2000", C2000Mode)
+                   .Case("cl2000", C2000Mode)
                    .Default(std::nullopt))
     Mode = *M;
   else
@@ -6971,7 +6971,7 @@ const char *Driver::getExecutableForDriverMode(DriverMode Mode) {
   case DXCMode:
     return "clang-dxc";
   case C2000Mode:
-  return "cl2000";
+    return "cl2000";
   }
 
   llvm_unreachable("Unhandled Mode");
diff --git a/llvm/include/llvm/TargetParser/Triple.h b/llvm/include/llvm/TargetParser/Triple.h
index 82e8e5a444..1fd966377d 100644
--- a/llvm/include/llvm/TargetParser/Triple.h
+++ b/llvm/include/llvm/TargetParser/Triple.h
@@ -46,63 +46,63 @@ public:
   enum ArchType {
     UnknownArch,
 
-    arm,            // ARM (little endian): arm, armv.*, xscale
-    armeb,          // ARM (big endian): armeb
-    aarch64,        // AArch64 (little endian): aarch64
-    aarch64_be,     // AArch64 (big endian): aarch64_be
-    aarch64_32,     // AArch64 (little endian) ILP32: aarch64_32
-    arc,            // ARC: Synopsys ARC
-    avr,            // AVR: Atmel AVR microcontroller
-    bpfel,          // eBPF or extended BPF or 64-bit BPF (little endian)
-    bpfeb,          // eBPF or extended BPF or 64-bit BPF (big endian)
-    c2000,           // C2000 MCU Family
-    csky,           // CSKY: csky
-    dxil,           // DXIL 32-bit DirectX bytecode
-    hexagon,        // Hexagon: hexagon
-    loongarch32,    // LoongArch (32-bit): loongarch32
-    loongarch64,    // LoongArch (64-bit): loongarch64
-    m68k,           // M68k: Motorola 680x0 family
-    mips,           // MIPS: mips, mipsallegrex, mipsr6
-    mipsel,         // MIPSEL: mipsel, mipsallegrexe, mipsr6el
-    mips64,         // MIPS64: mips64, mips64r6, mipsn32, mipsn32r6
-    mips64el,       // MIPS64EL: mips64el, mips64r6el, mipsn32el, mipsn32r6el
-    msp430,         // MSP430: msp430
-    ppc,            // PPC: powerpc
-    ppcle,          // PPCLE: powerpc (little endian)
-    ppc64,          // PPC64: powerpc64, ppu
-    ppc64le,        // PPC64LE: powerpc64le
-    r600,           // R600: AMD GPUs HD2XXX - HD6XXX
-    amdgcn,         // AMDGCN: AMD GCN GPUs
-    riscv32,        // RISC-V (32-bit): riscv32
-    riscv64,        // RISC-V (64-bit): riscv64
-    sparc,          // Sparc: sparc
-    sparcv9,        // Sparcv9: Sparcv9
-    sparcel,        // Sparc: (endianness = little). NB: 'Sparcle' is a CPU variant
-    systemz,        // SystemZ: s390x
-    tce,            // TCE (http://tce.cs.tut.fi/): tce
-    tcele,          // TCE little endian (http://tce.cs.tut.fi/): tcele
-    thumb,          // Thumb (little endian): thumb, thumbv.*
-    thumbeb,        // Thumb (big endian): thumbeb
-    x86,            // X86: i[3-9]86
-    x86_64,         // X86-64: amd64, x86_64
-    xcore,          // XCore: xcore
-    xtensa,         // Tensilica: Xtensa
-    nvptx,          // NVPTX: 32-bit
-    nvptx64,        // NVPTX: 64-bit
-    amdil,          // AMDIL
-    amdil64,        // AMDIL with 64-bit pointers
-    hsail,          // AMD HSAIL
-    hsail64,        // AMD HSAIL with 64-bit pointers
-    spir,           // SPIR: standard portable IR for OpenCL 32-bit version
-    spir64,         // SPIR: standard portable IR for OpenCL 64-bit version
-    spirv,          // SPIR-V with logical memory layout.
-    spirv32,        // SPIR-V with 32-bit pointers
-    spirv64,        // SPIR-V with 64-bit pointers
-    kalimba,        // Kalimba: generic kalimba
-    shave,          // SHAVE: Movidius vector VLIW processors
-    lanai,          // Lanai: Lanai 32-bit
-    wasm32,         // WebAssembly with 32-bit pointers
-    wasm64,         // WebAssembly with 64-bit pointers
+    arm,         // ARM (little endian): arm, armv.*, xscale
+    armeb,       // ARM (big endian): armeb
+    aarch64,     // AArch64 (little endian): aarch64
+    aarch64_be,  // AArch64 (big endian): aarch64_be
+    aarch64_32,  // AArch64 (little endian) ILP32: aarch64_32
+    arc,         // ARC: Synopsys ARC
+    avr,         // AVR: Atmel AVR microcontroller
+    bpfel,       // eBPF or extended BPF or 64-bit BPF (little endian)
+    bpfeb,       // eBPF or extended BPF or 64-bit BPF (big endian)
+    c2000,       // C2000 MCU Family
+    csky,        // CSKY: csky
+    dxil,        // DXIL 32-bit DirectX bytecode
+    hexagon,     // Hexagon: hexagon
+    loongarch32, // LoongArch (32-bit): loongarch32
+    loongarch64, // LoongArch (64-bit): loongarch64
+    m68k,        // M68k: Motorola 680x0 family
+    mips,        // MIPS: mips, mipsallegrex, mipsr6
+    mipsel,      // MIPSEL: mipsel, mipsallegrexe, mipsr6el
+    mips64,      // MIPS64: mips64, mips64r6, mipsn32, mipsn32r6
+    mips64el,    // MIPS64EL: mips64el, mips64r6el, mipsn32el, mipsn32r6el
+    msp430,      // MSP430: msp430
+    ppc,         // PPC: powerpc
+    ppcle,       // PPCLE: powerpc (little endian)
+    ppc64,       // PPC64: powerpc64, ppu
+    ppc64le,     // PPC64LE: powerpc64le
+    r600,        // R600: AMD GPUs HD2XXX - HD6XXX
+    amdgcn,      // AMDGCN: AMD GCN GPUs
+    riscv32,     // RISC-V (32-bit): riscv32
+    riscv64,     // RISC-V (64-bit): riscv64
+    sparc,       // Sparc: sparc
+    sparcv9,     // Sparcv9: Sparcv9
+    sparcel,     // Sparc: (endianness = little). NB: 'Sparcle' is a CPU variant
+    systemz,     // SystemZ: s390x
+    tce,         // TCE (http://tce.cs.tut.fi/): tce
+    tcele,       // TCE little endian (http://tce.cs.tut.fi/): tcele
+    thumb,       // Thumb (little endian): thumb, thumbv.*
+    thumbeb,     // Thumb (big endian): thumbeb
+    x86,         // X86: i[3-9]86
+    x86_64,      // X86-64: amd64, x86_64
+    xcore,       // XCore: xcore
+    xtensa,      // Tensilica: Xtensa
+    nvptx,       // NVPTX: 32-bit
+    nvptx64,     // NVPTX: 64-bit
+    amdil,       // AMDIL
+    amdil64,     // AMDIL with 64-bit pointers
+    hsail,       // AMD HSAIL
+    hsail64,     // AMD HSAIL with 64-bit pointers
+    spir,        // SPIR: standard portable IR for OpenCL 32-bit version
+    spir64,      // SPIR: standard portable IR for OpenCL 64-bit version
+    spirv,       // SPIR-V with logical memory layout.
+    spirv32,     // SPIR-V with 32-bit pointers
+    spirv64,     // SPIR-V with 64-bit pointers
+    kalimba,     // Kalimba: generic kalimba
+    shave,       // SHAVE: Movidius vector VLIW processors
+    lanai,       // Lanai: Lanai 32-bit
+    wasm32,      // WebAssembly with 32-bit pointers
+    wasm64,      // WebAssembly with 64-bit pointers
     renderscript32, // 32-bit RenderScript
     renderscript64, // 64-bit RenderScript
     ve,             // NEC SX-Aurora Vector Engine
diff --git a/llvm/lib/TargetParser/Triple.cpp b/llvm/lib/TargetParser/Triple.cpp
index 0e3f2cfa42..1479a75c40 100644
--- a/llvm/lib/TargetParser/Triple.cpp
+++ b/llvm/lib/TargetParser/Triple.cpp
@@ -37,7 +37,8 @@ StringRef Triple::getArchTypeName(ArchType Kind) {
   case avr:            return "avr";
   case bpfeb:          return "bpfeb";
   case bpfel:          return "bpfel";
-  case c2000:          return "c2000";
+  case c2000:
+    return "c2000";
   case csky:           return "csky";
   case dxil:           return "dxil";
   case hexagon:        return "hexagon";
@@ -556,85 +557,84 @@ static Triple::ArchType parseARMArch(StringRef ArchName) {
 }
 
 static Triple::ArchType parseArch(StringRef ArchName) {
-  auto AT =
-      StringSwitch<Triple::ArchType>(ArchName)
-          .Cases("i386", "i486", "i586", "i686", Triple::x86)
-          // FIXME: Do we need to support these?
-          .Cases("i786", "i886", "i986", Triple::x86)
-          .Cases("amd64", "x86_64", "x86_64h", Triple::x86_64)
-          .Cases("powerpc", "powerpcspe", "ppc", "ppc32", Triple::ppc)
-          .Cases("powerpcle", "ppcle", "ppc32le", Triple::ppcle)
-          .Cases("powerpc64", "ppu", "ppc64", Triple::ppc64)
-          .Cases("powerpc64le", "ppc64le", Triple::ppc64le)
-          .Case("xscale", Triple::arm)
-          .Case("xscaleeb", Triple::armeb)
-          .Case("aarch64", Triple::aarch64)
-          .Case("aarch64_be", Triple::aarch64_be)
-          .Case("aarch64_32", Triple::aarch64_32)
-          .Case("arc", Triple::arc)
-          .Case("arm64", Triple::aarch64)
-          .Case("arm64_32", Triple::aarch64_32)
-          .Case("arm64e", Triple::aarch64)
-          .Case("arm64ec", Triple::aarch64)
-          .Case("arm", Triple::arm)
-          .Case("armeb", Triple::armeb)
-          .Case("thumb", Triple::thumb)
-          .Case("thumbeb", Triple::thumbeb)
-          .Case("avr", Triple::avr)
-          .Case("m68k", Triple::m68k)
-          .Case("msp430", Triple::msp430)
-          .Case("c2000", Triple::c2000)
-          .Cases("mips", "mipseb", "mipsallegrex", "mipsisa32r6", "mipsr6",
-                 Triple::mips)
-          .Cases("mipsel", "mipsallegrexel", "mipsisa32r6el", "mipsr6el",
-                 Triple::mipsel)
-          .Cases("mips64", "mips64eb", "mipsn32", "mipsisa64r6", "mips64r6",
-                 "mipsn32r6", Triple::mips64)
-          .Cases("mips64el", "mipsn32el", "mipsisa64r6el", "mips64r6el",
-                 "mipsn32r6el", Triple::mips64el)
-          .Case("r600", Triple::r600)
-          .Case("amdgcn", Triple::amdgcn)
-          .Case("riscv32", Triple::riscv32)
-          .Case("riscv64", Triple::riscv64)
-          .Case("hexagon", Triple::hexagon)
-          .Cases("s390x", "systemz", Triple::systemz)
-          .Case("sparc", Triple::sparc)
-          .Case("sparcel", Triple::sparcel)
-          .Cases("sparcv9", "sparc64", Triple::sparcv9)
-          .Case("tce", Triple::tce)
-          .Case("tcele", Triple::tcele)
-          .Case("xcore", Triple::xcore)
-          .Case("nvptx", Triple::nvptx)
-          .Case("nvptx64", Triple::nvptx64)
-          .Case("amdil", Triple::amdil)
-          .Case("amdil64", Triple::amdil64)
-          .Case("hsail", Triple::hsail)
-          .Case("hsail64", Triple::hsail64)
-          .Case("spir", Triple::spir)
-          .Case("spir64", Triple::spir64)
-          .Cases("spirv", "spirv1.5", "spirv1.6", Triple::spirv)
-          .Cases("spirv32", "spirv32v1.0", "spirv32v1.1", "spirv32v1.2",
-            "spirv32v1.3", "spirv32v1.4", "spirv32v1.5",
-            "spirv32v1.6", Triple::spirv32)
-          .Cases("spirv64", "spirv64v1.0", "spirv64v1.1", "spirv64v1.2",
-            "spirv64v1.3", "spirv64v1.4", "spirv64v1.5",
-            "spirv64v1.6", Triple::spirv64)
-          .StartsWith("kalimba", Triple::kalimba)
-          .Case("lanai", Triple::lanai)
-          .Case("renderscript32", Triple::renderscript32)
-          .Case("renderscript64", Triple::renderscript64)
-          .Case("shave", Triple::shave)
-          .Case("ve", Triple::ve)
-          .Case("wasm32", Triple::wasm32)
-          .Case("wasm64", Triple::wasm64)
-          .Case("csky", Triple::csky)
-          .Case("loongarch32", Triple::loongarch32)
-          .Case("loongarch64", Triple::loongarch64)
-          .Cases("dxil", "dxilv1.0", "dxilv1.1", "dxilv1.2", "dxilv1.3",
-                 "dxilv1.4", "dxilv1.5", "dxilv1.6", "dxilv1.7", "dxilv1.8",
-                 Triple::dxil)
-          .Case("xtensa", Triple::xtensa)
-          .Default(Triple::UnknownArch);
+  auto AT = StringSwitch<Triple::ArchType>(ArchName)
+                .Cases("i386", "i486", "i586", "i686", Triple::x86)
+                // FIXME: Do we need to support these?
+                .Cases("i786", "i886", "i986", Triple::x86)
+                .Cases("amd64", "x86_64", "x86_64h", Triple::x86_64)
+                .Cases("powerpc", "powerpcspe", "ppc", "ppc32", Triple::ppc)
+                .Cases("powerpcle", "ppcle", "ppc32le", Triple::ppcle)
+                .Cases("powerpc64", "ppu", "ppc64", Triple::ppc64)
+                .Cases("powerpc64le", "ppc64le", Triple::ppc64le)
+                .Case("xscale", Triple::arm)
+                .Case("xscaleeb", Triple::armeb)
+                .Case("aarch64", Triple::aarch64)
+                .Case("aarch64_be", Triple::aarch64_be)
+                .Case("aarch64_32", Triple::aarch64_32)
+                .Case("arc", Triple::arc)
+                .Case("arm64", Triple::aarch64)
+                .Case("arm64_32", Triple::aarch64_32)
+                .Case("arm64e", Triple::aarch64)
+                .Case("arm64ec", Triple::aarch64)
+                .Case("arm", Triple::arm)
+                .Case("armeb", Triple::armeb)
+                .Case("thumb", Triple::thumb)
+                .Case("thumbeb", Triple::thumbeb)
+                .Case("avr", Triple::avr)
+                .Case("m68k", Triple::m68k)
+                .Case("msp430", Triple::msp430)
+                .Case("c2000", Triple::c2000)
+                .Cases("mips", "mipseb", "mipsallegrex", "mipsisa32r6",
+                       "mipsr6", Triple::mips)
+                .Cases("mipsel", "mipsallegrexel", "mipsisa32r6el", "mipsr6el",
+                       Triple::mipsel)
+                .Cases("mips64", "mips64eb", "mipsn32", "mipsisa64r6",
+                       "mips64r6", "mipsn32r6", Triple::mips64)
+                .Cases("mips64el", "mipsn32el", "mipsisa64r6el", "mips64r6el",
+                       "mipsn32r6el", Triple::mips64el)
+                .Case("r600", Triple::r600)
+                .Case("amdgcn", Triple::amdgcn)
+                .Case("riscv32", Triple::riscv32)
+                .Case("riscv64", Triple::riscv64)
+                .Case("hexagon", Triple::hexagon)
+                .Cases("s390x", "systemz", Triple::systemz)
+                .Case("sparc", Triple::sparc)
+                .Case("sparcel", Triple::sparcel)
+                .Cases("sparcv9", "sparc64", Triple::sparcv9)
+                .Case("tce", Triple::tce)
+                .Case("tcele", Triple::tcele)
+                .Case("xcore", Triple::xcore)
+                .Case("nvptx", Triple::nvptx)
+                .Case("nvptx64", Triple::nvptx64)
+                .Case("amdil", Triple::amdil)
+                .Case("amdil64", Triple::amdil64)
+                .Case("hsail", Triple::hsail)
+                .Case("hsail64", Triple::hsail64)
+                .Case("spir", Triple::spir)
+                .Case("spir64", Triple::spir64)
+                .Cases("spirv", "spirv1.5", "spirv1.6", Triple::spirv)
+                .Cases("spirv32", "spirv32v1.0", "spirv32v1.1", "spirv32v1.2",
+                       "spirv32v1.3", "spirv32v1.4", "spirv32v1.5",
+                       "spirv32v1.6", Triple::spirv32)
+                .Cases("spirv64", "spirv64v1.0", "spirv64v1.1", "spirv64v1.2",
+                       "spirv64v1.3", "spirv64v1.4", "spirv64v1.5",
+                       "spirv64v1.6", Triple::spirv64)
+                .StartsWith("kalimba", Triple::kalimba)
+                .Case("lanai", Triple::lanai)
+                .Case("renderscript32", Triple::renderscript32)
+                .Case("renderscript64", Triple::renderscript64)
+                .Case("shave", Triple::shave)
+                .Case("ve", Triple::ve)
+                .Case("wasm32", Triple::wasm32)
+                .Case("wasm64", Triple::wasm64)
+                .Case("csky", Triple::csky)
+                .Case("loongarch32", Triple::loongarch32)
+                .Case("loongarch64", Triple::loongarch64)
+                .Cases("dxil", "dxilv1.0", "dxilv1.1", "dxilv1.2", "dxilv1.3",
+                       "dxilv1.4", "dxilv1.5", "dxilv1.6", "dxilv1.7",
+                       "dxilv1.8", Triple::dxil)
+                .Case("xtensa", Triple::xtensa)
+                .Default(Triple::UnknownArch);
 
   // Some architectures require special parsing logic just to compute the
   // ArchType result.

@DragonDisciple
Copy link
Contributor

Hello, any clang/llvm people who take a look at this! I'm J.B. Nagurne, and I'm a member of the compiler team for Texas Instruments. I have experience with both the TI proprietary compiler (here, cl2000) and the LLVM-based TI compilers, c29clang and tiarmclang.

I'll be reviewing of this code, but will hold off on allowing a merge until someone external to TI signs off as well.

@DragonDisciple
Copy link
Contributor

DragonDisciple commented Feb 4, 2025

General comments:

  • Please do not use tabs for spacing. This is what causes the unexpected whitespace gaps in the diff.
  • There is a utility, clang-format-diff.py, which will assist in appeasing the code formatting check. However, I noticed running it myself that it changes some entire lists that should likely stay the way they are, so be careful!
    • My usual command line is something like: git diff -U0 --no-color --relative HEAD~1..HEAD | /path/to/clang-format-diff.py -p1 -i -binary /path/to/clang-format(.exe)
  • Is there no desire to catch and support C2000 built-in functions? Those will show up in clangd as undefined symbols and will be errors in c99 or later, and in all C++ modes.
  • Options
    • The options parser for clang and for the TI compilers are obviously very different. Some options interact with others for the purposes of ease-of-use and error checking, which is something that the clang options parser won't understand. I'll do my best to be diligent in case I have to point this out
    • There are many more options than the ones currently captured. Is it important we get as many as we can? Otherwise this will be a PR that is tailor-fit to a single use-case.
  • Is there a testing strategy you have in mind? At the very least, we need to cover the selection of the new mode and options. See clang/test/Driver/cl-options.c for an example

HelpText<"Treat source input files as Objective-C inputs">;
def O : Joined<["-"], "O">, Group<O_Group>,
Visibility<[ClangOption, CC1Option, FC1Option, FlangOption]>;
Visibility<[ClangOption, CC1Option, FC1Option, FlangOption, CL2000Option]>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The C2000 compiler accepts '-o' or '--opt_level' to define the optimization level as well.

This overlaps with clang's -o outfile specification, so there would need to be some sort of translation from TI's --output_file to clang's -o. Perhaps parsing the option would be enough, since there is no output from clangd?




def cl2000_include_path : Joined<["--"], "include_path=">, Group<cl2000_group>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the use-case for this particular set of options? I'd suggest that options which may change internal definitions or affect the parse should be here. Perhaps everything else can somehow be ignored silently.

  • -ml for large_model is shorthand for --large_memory_model, but that's not present in this list.
  • --preinclude=, which should be identical to 'include_' above
  • --define and --undefine for -D and -U, respectively
  • Language options --c89, --c99, --c1, --c++03
  • --cpp_default (-fg), which is similar to (but not identical to) -x cpp
  • --c_file/-fc and --cpp_file/-fp to set single file language types

These are mostly presented as examples. If only a subset of these options are merged, then I fear that the commit becomes very narrow in its use-case. If upstream is fine with that, then I'd be fine too.

Copy link
Author

@student433 student433 Feb 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand, this change was adhered specifically to our use case and it was diagnosed that all other flags got ignored silently once only the first few in compile commands were defined here, but of course, that is an individual case, so it would be better to add all flags for future.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am currently about to add the language options support, but would like to know how to go about defining the strict_ansi and relaxed_ansi options. These disable the GNU extensions and also enable some TI specific keyword usage in code like interrupt, and also go with the other c/c++ language options. Would be nice to have a path if you already know :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strict_ansi/relaxed_ansi should map to the clang -std option: -std=c99 is strict mode, -std=gnu99 is relaxed mode. The difference is small enough that you could probably just ignore the options, though.

using namespace clang::targets;

const char *const C2000TargetInfo::GCCRegNames[] = {
"ACC", "XAR0", "XAR1", "XAR2", "XAR3", "XAR4", "XAR5", "XAR6", "XAR7"};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was there a reason for defining this? The TI compilers don't support extended assembly syntax, which is what this list is used for, if I remember correctly.

Copy link
Author

@student433 student433 Feb 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right! This is somewhere extended to codegen and support for this syntax could be omitted if TI compilers do not support them as clangd would have no issues, it was just added for fulfillment and a possible future support in codegen


// Assembly Instrinsics

Builder.append("int __abs16_sat( int src );");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I complained about builtins but I did so looking at the source files. I didn't know you could do this...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No other target does this, so I'll need to have someone upstream clear it for commit.
Otherwise, you'd need to go through the normal procedure of defining a Builtins tablegen file for C2000. The only difference is you won't need a translation to IR Intrinsic.

Copy link
Author

@student433 student433 Feb 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I was aware of this and wanted a clear direction before adding too many changes :) So if you or upstream suggest, I can look into adding the builtins tabelgen file for cl2000.

Builder.append("void *memcpy(void * __restrict s1, const void * __restrict "
"s2, unsigned long n);");
Builder.defineMacro("__intaddr__(x)", "x");
Builder.defineMacro("asm(x)", "");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is... dangerous, since asm is a semantic statement that you're just going to make disappear.
Again, would need a comment upstream for this.

// Fast Integer Division Intrinsics
if (hasFeature("idiv-support")) {
Builder.append(
"ldiv_t __traditional_div_i16byi16( int dividend, int divisor );");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This declaration and everything below use some flavor (standard and otherwise) of div_t structure. However, nothing in this pseudo-source would declare those objects.

In the library implementation, available in the installation of stdlib.h, I find that all the div_t flavors are defined in stdlib.h guarded by the macro "__TMS320C28XX_IDIV__'.

This brings up another question: which libc will clangd use? If it uses the host's version, then I foresee many more issues popping up related to missing declarations or declarations that don't quite match.

If it uses the TI C library, then I foresee many more issues popping up related to the assertion that this target is 'cl2000'. Take for example the macro _TI_PROPRIETARY_PRAGMA, which is defined in such a way as to be ignored by clang-based compilers but become a TI-only pragma when '__TI_COMPILER_VERSION__" is defined, as it is above. Perhaps clangd will ignore it silently? I don't know...

There are many more uses of "__TI_COMPILER_VERSION__" in our libc that may cause issue, but I haven't dug so deep on those.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this! I have fixed the idiv-support condition now.

For your second feedback, -cc1 understands every include directory as an isystem, just as cl2000 doesn't differentiate between a standard header and a user defined one, therefore the TI library is picked up by clang. Currently, we use a blank placeholder for defining __TI_COMPILER_VERSION__ and could also enable the pragma macro if __TI_COMPILER_VERSION__ is defined without any conditions. I have little knowledge about this in depth, so please feel free to guide.

public:
C2000TargetInfo(const llvm::Triple &Triple, const TargetOptions &)
: TargetInfo(Triple) {
TLSSupported = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that CharWidth isn't modified to 16 here to avoid an assertion. The C2000 ISA is 16-bit addressable.

Copy link
Author

@student433 student433 Feb 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, since llvm currently doesn't support 16-bit addressability, and is hardcoded to "8", i just resorted to undefining the existing __CHAR_BIT__ macro and redefining it as 16 as you saw. There already exists a FIXME at line 509 in clang/include/clang/Basic/TargetInfo.h. So I would hope that upstream gives either leeway or suggests a compliant solution for this issue.

ArrayRef<const char *> getGCCRegNames() const override;

ArrayRef<TargetInfo::GCCRegAlias> getGCCRegAliases() const override {
// Make these be recognized by llc (f.e., in clobber list)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy-pasted and should likely be an empty set instead. 'llc' should never see code generated by this target.

}
} else if (IsC2000Mode()) {
llvm::Triple T(TargetTriple);
T.setArch(llvm::Triple::c2000);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not so sure about the Arch/Vendor. The architecture itself is the 'C28X' or 'C28', so 'c28' makes more sense than C2000.
As for vendor, maybe adding a TI vendor would be preferred? I'm not sure if there's a decorum for that field. We use c29-ti and arm-ti for our own toolchains.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also thought this would make sense and I think currently a 4 component(arch, vendor, OS, env) string is created for triple. Maybe c28-ti-unknown-eabi makes more sense then, if it also makes sense to upstream.

#include <string>
#include <vector>

namespace clang {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think upstream prefers the modern nested namespace declaration, but I suppose no other Arch file does that currently! There are examples of it around code, though. Cosmetic and just saves some space.

namespace clang::driver::tools::c2000

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it! Modifying this for better compatibility

@kadircet kadircet changed the title [clangd] Add support for the c2000 architecture [clang] Add support for the c2000 architecture Feb 5, 2025
@kadircet kadircet requested a review from AaronBallman February 5, 2025 07:53
@SonicStark
Copy link

Excited to see your nice work!!!

AFAIK all TI extension keywords may confuse clang frontend, and treating them as macros would result in strange behaviors, e.g.

// DSP2833x_PieVect.h
typedef interrupt void(*PINT)(void);

So special keywords handling would be necessary...

See SPRU514Z - TMS320C28x Optimizing C/C++ Compiler:

6.5 Keywords
The C28x C/C++ compiler supports all of the standard C89 keywords, including const, volatile, and register. It supports all of the standard C99 keywords, including inline and restrict. It supports all of the standard C11 keywords. It also supports TI extension keywords __interrupt, __cregister, and __asm.
...
6.5.2 The __cregister Keyword
...
6.5.3 The __interrupt Keyword
...

CMIIW :) @DragonDisciple

@student433
Copy link
Author

General comments:

* Please do not use tabs for spacing. This is what causes the unexpected whitespace gaps in the diff.

* There is a utility, clang-format-diff.py, which will assist in appeasing the code formatting check. However, I noticed running it myself that it changes some entire lists that should likely stay the way they are, so be careful!
  
  * My usual command line is something like: git diff -U0 --no-color --relative HEAD~1..HEAD | /path/to/clang-format-diff.py -p1 -i -binary /path/to/clang-format(.exe)

* Is there no desire to catch and support C2000 built-in functions? Those will show up in clangd as undefined symbols and will be errors in c99 or later, and in all C++ modes.

* Options
  
  * The options parser for clang and for the TI compilers are obviously very different. Some options interact with others for the purposes of ease-of-use and error checking, which is something that the clang options parser won't understand. I'll do my best to be diligent in case I have to point this out
  * There are many more options than the ones currently captured. Is it important we get as many as we can? Otherwise this will be a PR that is tailor-fit to a single use-case.

* Is there a testing strategy you have in mind? At the very least, we need to cover the selection of the new mode and options. See clang/test/Driver/cl-options.c for an example

Thank you very much for your inputs,

I have yet to add the test cases for cl2000 driver in clang/tests/Driver and pair it with diagnostics as needed.

I will be sure to check the clang-format for indentation and other things. Will respond to each comment individually :)

Excited to see your nice work!!!

AFAIK all TI extension keywords may confuse clang frontend, and treating them as macros would result in strange behaviors, e.g.

// DSP2833x_PieVect.h
typedef interrupt void(*PINT)(void);

So special keywords handling would be necessary...

See SPRU514Z - TMS320C28x Optimizing C/C++ Compiler:

6.5 Keywords
The C28x C/C++ compiler supports all of the standard C89 keywords, including const, volatile, and register. It supports all of the standard C99 keywords, including inline and restrict. It supports all of the standard C11 keywords. It also supports TI extension keywords __interrupt, __cregister, and __asm.
...
6.5.2 The __cregister Keyword
...
6.5.3 The __interrupt Keyword
...

CMIIW :) @DragonDisciple

Thank you very much for your inputs, I understand that these are semantically incorrect and would require a meaningful definition to avoid it being misunderstood. I could look into this :)

@efriedma-quic
Copy link
Collaborator

If you wanted to do things "properly", you'd probably define __interrupt and __cregister as attributes in clang/include/clang/Basic/Attr.td . But at first glance, using macros to ignore __interrupt and __cregister behaves basically correctly, if you're not actually trying to generate code. So probably not worth the trouble.

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Marking this as requesting changes so we don't accidentally land it. The discussion on the RFC is still going and there may not be consensus to move forward with this PR, so I'd like to see that discussion resolved before we land anything.

def cl2000_include_path : Joined<["--"], "include_path=">, Group<cl2000_group>,
HelpText<"specify include search paths for cl2000 driver mode">, Alias<isystem>;

def eabi : Joined<["--"], "abi=">, Group<cl2000_group>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am concerned of adding driver options (perhaps mostly ignored). A better way is to change language servers and clang-tidy to ignore options. These tools can simply set an environment variable

CCC_OVERRIDE_OPTIONS=#x-fxxx clang -fsyntax-only -fxxx /tmp/t/a.c   # does not error on -fxxx

Copy link
Member

@MaskRay MaskRay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(strongly object to the driver mode and option changes. But thanks for proposing the pseudo target RFC, which has potential to support many downstream targets without more custom code in Basic/Targets/Driver.)

(Request changes to ensure that I'll have a chance to look again when this is accepted by others.)

// `flang-new`. This will be removed in the future.
{"flang-new", "--driver-mode=flang"},
{"clang-dxc", "--driver-mode=dxc"},
{"cl2000", "--driver-mode=cl2000"},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I strongly object to introduce a new driver mode without a very active community and full-blown support (including codegen). You can see taht all the existing ones are not added lightly. There are very heavy commitment on the maintenance (e.g. flang, dxc). It would not significant maintenance nightmare if every downstream target without codegen support adds a driver mode here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TI (Texas Instruments) C2000 Family not supported by clangd language server

7 participants