[PATCH] D32669: [Nios2] Target registration

[Andrei via Phabricator via llvm-commits]

AndreiGrischenko added inline comments.


================
Comment at: ../llvm/CMakeLists.txt:315
   MSP430
+  Nios2
   NVPTX
----------------
craig.topper wrote:
> 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.
Yes, I assume at the moment we do not want that. I will remove Nios2 from there.


================
Comment at: ../llvm/lib/Target/Nios2/CMakeLists.txt:1
+set(LLVM_TARGET_DEFINITIONS Nios2.td)
+
----------------
DavidKreitzer wrote:
> Is there a clear LLVM policy regarding these svn attributes? The repository is pretty inconsistent, but you should at least be consistent within your patch. I suspect that you want to remove them.
> 
I do not know about that policy. And yes I will remove these attributes.


================
Comment at: ../llvm/lib/Target/Nios2/LLVMBuild.txt:49
+#the tool needs.
+required_libraries = CodeGen
+                     Core
----------------
craig.topper wrote:
> 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.
OK


================
Comment at: ../llvm/lib/Target/Nios2/MCTargetDesc/Nios2MCTargetDesc.h:1
+//===-- Nios2MCTargetDesc.h - Nios2 Target Descriptions -----------*- C++
+//-*-===//
----------------
craig.topper wrote:
> 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.
OK, I will fix that


================
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";
+
----------------
craig.topper wrote:
> Can we just return the string without the temporary variable?
Sure. Thanks.


https://reviews.llvm.org/D32669