[PATCH] D32669: [Nios2] Target registration

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: ../llvm/CMakeLists.txt:315
   MSP430
+  Nios2
   NVPTX
----------------
Do we want this as part of the default build yet? Or should it be experimental and optin for now? WebAssembly for example isn't listed here and must be explicitly built by passing it to cmake.


================
Comment at: ../llvm/lib/Target/Nios2/LLVMBuild.txt:49
+#the tool needs.
+required_libraries = CodeGen
+                     Core
----------------
I think these should be in alphabetical order. That's consistent with what other targets did before GlobalISel started getting added to the end of everyone's list.


================
Comment at: ../llvm/lib/Target/Nios2/MCTargetDesc/Nios2MCTargetDesc.h:1
+//===-- Nios2MCTargetDesc.h - Nios2 Target Descriptions -----------*- C++
+//-*-===//
----------------
Don't line wrap this first line comment.  -*- C++ -*-  is a magic token for text editors and needs to be together. This applies to several files in this review.


================
Comment at: ../llvm/lib/Target/Nios2/Nios2TargetMachine.cpp:28
+                                     const TargetOptions &Options) {
+  std::string Ret = "e-p:32:32:32-i8:8:32-i16:16:32-n32";
+
----------------
Can we just return the string without the temporary variable?


https://reviews.llvm.org/D32669