[PATCH] Add nominal support for 'shave' target.

Douglas Katzman dougk at google.com
Wed Jun 17 07:32:07 PDT 2015


================
Comment at: lib/Driver/Driver.cpp:1572
@@ -1571,3 +1571,3 @@
       Result = InputInfo(Name, A->getType(), Name);
-    } else
+    } else {
       Result = InputInfo(&Input, A->getType(), "");
----------------
joerg wrote:
> Unrelated?
committed separately

================
Comment at: lib/Driver/ToolChains.cpp:3748
@@ +3747,3 @@
+
+bool MoviClang::isPICDefault() const { return true; }
+bool MoviClang::isPIEDefault() const { return false; }
----------------
joerg wrote:
> No need to make those out of line?
Seems these can be omitted. Since SHAVEToolChain inherits from Generic_GCC, it finds methods for isBlahDefault() and printVerboseInfo().  Is that OK?

================
Comment at: lib/Driver/ToolChains.h:875-877
@@ -874,1 +874,5 @@
 
+/// MoviClang - A tool chain using the compiler that the SDK installs
+/// into MV_TOOLS_DIR (copied to llvm's installdir) to perform all subcommands.
+class LLVM_LIBRARY_VISIBILITY MoviClang : public Generic_GCC {
+public:
----------------
chandlerc wrote:
> I'm not a huge fan of the "Movi" prefix. Maybe just "Movidius" or "SHAVE"? If this is only used for the shave-* triples, I'd probably go with SHAVE.
> 
> I would also suffix it with ToolChain or something. It's not about Clang, its about the toolchain. (The use of 'GCC' here is because GCC actually ships a complete toolchain)
SHAVEToolChain is fine with me. But Hexagon abbreviates to Hexagon_TC, and XCore uses no suffix. We should standardize on _TC or ToolChain. Which?

================
Comment at: lib/Driver/ToolChains.h:895-896
@@ +894,4 @@
+private:
+  mutable std::unique_ptr<Tool> moviCompile;
+  mutable std::unique_ptr<Tool> moviAsm;
+};
----------------
chandlerc wrote:
> I suspect these should be named MoviCompile and MoviAsm. I would also probably call these "Compiler" and "Assembler" because I'm a bit of a pedant. ;]
Sure, but it introduces gratuitous inconsistency.

I also liked Compiler and Assembler except that Generic_GCC names its member variables 'Preprocess' and 'Compile' rather than 'Preprocessor' and 'Compiler'.

And the method to make an instance of tool model is tools::gcc::Preprocess (respectively tools::gcc::Compile). Those are highly un-mnemonic. They really sound like they mean *do* that job, but what they mean is "give me a thing that does that job".

And while we're on the subject, anything named someToolChain::Assemble::ConstructJob should be renamed to someToolChain::Assembler::ConstructJob, and the same for all other tools.



================
Comment at: lib/Driver/Tools.h:734-735
@@ -733,1 +733,4 @@
 
+/// movitool -- Directly call moviCompile and moviAsm
+namespace movitool {
+class LLVM_LIBRARY_VISIBILITY Compile : public Tool {
----------------
chandlerc wrote:
> Similar to the above, I'd probably call this SHAVE given that some of the ones above are XCore.
done

http://reviews.llvm.org/D10440

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list