vkalintiris added inline comments. ================ Comment at: include/clang/Driver/ToolChain.h:147 @@ -145,1 +146,3 @@ + const Multilib &getSelectedMultilib() const { return SelectedMultilib; } + ---------------- atanasyan wrote: > Do you need public access to this member function? I discarded this member function after the move of `SelectedMultilib` to the `MipsToolChain` class.
================ Comment at: lib/Driver/Driver.cpp:2127 @@ +2126,3 @@ + // Allow the discovery of tools prefixed with LLVM's default target triple. + std::string LLVMDefaultTargetTriple = llvm::sys::getDefaultTargetTriple(); + if (LLVMDefaultTargetTriple != DefaultTargetTriple) ---------------- atanasyan wrote: > Just curious when is DefaultTargetTriple not equal to LLVMDefaultTargetTriple? In this case, `DefaultTargetTriple` is the triple specified with `--target=`. LLVMDefaultTargetTriple is the value specified with the CMake variable `-DLLVM_DEFAULT_TARGET_TRIPLE=` during configuration time. For example, using `--target=mips64el-mti-linux` will search for files prefixed with either `mips64el-mti-linux-{as,ld}` and `mips-mti-linux-{as,ld}` in our toolchain where we specify `-DLLVM_DEFAULT_TARGET_TRIPLE=mips-mti-linux.` ================ Comment at: lib/Driver/Driver.cpp:2225 @@ -2219,1 +2224,3 @@ TC = new toolchains::HexagonToolChain(*this, Target, Args); + else if (Target.getVendor() == llvm::Triple::MipsTechnologies) + TC = new toolchains::MipsToolChain(*this, Target, Args); ---------------- atanasyan wrote: > The `mips-mti-linux-gnu` triple is used by Codescape toolchain too. After > this change if user provides `-target mips-mti-linux-gnu` command line > option, the `MipsToolChain` will be used. As far as I understand you have to > put `GCCInstallation.isValid()` checking to the `MipsToolChain` class methods > to allow working with both GNU and non-GNU toolchains. IMHO it does not make > the code clear. Maybe use the `MipsToolChain` class for the non-GNU toolchain > only. Shall we change the `MipsToolChain` class name to reflect these changes? Maybe `MipsNonGNUToolChain`? It would be strange to keep the same name, because most of the MIPS toolchains wouldn't be instantiated by `MipsToolChain`. ================ Comment at: lib/Driver/ToolChains.cpp:2231 @@ +2230,3 @@ + // If we did find a valid GCC installation, we don't have anything else to do. + if (GCCInstallation.isValid()) + return; ---------------- atanasyan wrote: > When is GCCInstallation invalid in case of using this toolchain? I changed that, according to your comments earlier. Now we instantiate this class only for triples that don't have an environment. http://reviews.llvm.org/D13340 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits