dougk abandoned this revision.
================
Comment at: lib/Driver/Tools.cpp:9110
@@ +9109,3 @@
+
+ const char *ToolsRoot = ::getenv("MV_TOOLS_DIR");
+ // The version is numbered 'n.n.n.n' for arbitrary values that are opaque
----------------
jyknight wrote:
> Perhaps this should be a default plus a command-line argument, instead of an
> environment variable?
How about if '--sysdir' provides the value, falling back to the environment
var, and then a fixed default?
There is ample precedent for use of getenv() in some of the other toolchains.
================
Comment at: lib/Driver/Tools.cpp:9144
@@ +9143,3 @@
+ ToolsRoot = "/usr/local/myriad/tools";
+ ToolsVersion = "00.50.62.5";
+ ToolsCommonDir = "/usr/local/mdk/tools/" + ToolsVersion + "/common";
----------------
jyknight wrote:
> Doesn't seem right to hardcode this particular version. Using a default for
> ToolsRoot if MV_TOOLS_DIR isn't set sounds okay, but presumably should be
> done up above before the loop.
>
I could see a couple ways around hardcoding a version - if neither --sysroot
nor MV_TOOLS_DIR is used, then the path is arbitrary anyway, so doesn't need a
versioned subdir. And arguably is sysroot is given, then it could be an
already-versioned pathname. Otoh, detecting the version works fine in that case.
For reference, the example sub-Makefile has hardcoded:
MV_TOOLS_VERSION ?= 00.50.62.5
MV_TOOLS_DIR ?= $(HOME)/WORK/Tools/Releases/General
================
Comment at: lib/Driver/Tools.cpp:9160
@@ +9159,3 @@
+ CmdArgs.push_back("-EL"); // Endianness = little
+ CmdArgs.push_back("-O9"); // Optimization
+ CmdArgs.push_back("--gc-sections");
----------------
jyknight wrote:
> "-O9"? That seems weird, and is not actually a thing.
This was cargo-cultism. I've no objection to taking it out, but who's to say
that their linker doesn't use it for something, as why else would they bother
to put it in the examples?
http://reviews.llvm.org/D10841
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
