[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