I agree with Joerg about submitting the separate cleanup change separately 
(feel free to just pull it out and commit it).

More detailed comments below.


================
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:
----------------
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)

================
Comment at: lib/Driver/ToolChains.h:895-896
@@ +894,4 @@
+private:
+  mutable std::unique_ptr<Tool> moviCompile;
+  mutable std::unique_ptr<Tool> moviAsm;
+};
----------------
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. ;]

================
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 {
----------------
Similar to the above, I'd probably call this SHAVE given that some of the ones 
above are XCore.

http://reviews.llvm.org/D10440

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



_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to