atanasyan added inline comments. ================ Comment at: include/clang/Driver/ToolChain.h:92 @@ -91,2 +91,3 @@ MultilibSet Multilibs; + Multilib SelectedMultilib; ---------------- This field is used by the `MipsToolChain` class only. If so, move it to that class.
================ Comment at: include/clang/Driver/ToolChain.h:147 @@ -145,1 +146,3 @@ + const Multilib &getSelectedMultilib() const { return SelectedMultilib; } + ---------------- Do you need public access to this member function? ================ 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) ---------------- Just curious when is DefaultTargetTriple not equal to LLVMDefaultTargetTriple? ================ 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); ---------------- 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. ================ Comment at: lib/Driver/ToolChains.cpp:2210 @@ -2172,2 +2209,3 @@ const Generic_GCC::GCCVersion &V = GCCInstallation.getVersion(); + const llvm::Triple &TT = getTriple(); bool UseInitArrayDefault = ---------------- Let's make this change by a separate commit to reduce number of unrelated changes. ================ 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; ---------------- When is GCCInstallation invalid in case of using this toolchain? ================ Comment at: lib/Driver/ToolChains.cpp:2243 @@ +2242,3 @@ + tools::mips::getMipsCPUAndABI(Args, Triple, CPUName, ABIName); + LibSuffix = llvm::StringSwitch<std::string>(ABIName) + .Case("o32", "") ---------------- Similar code exists in the `getLinuxDynamicLinker` routine. Let's factor out it into the separate function say `tools::mips::getMipsABILibSuffix`. ================ Comment at: lib/Driver/Tools.cpp:8115 @@ -8110,2 +8114,3 @@ const llvm::Triple::ArchType Arch = ToolChain.getArch(); + const llvm::Triple &TT = ToolChain.getTriple(); ---------------- Let's make this change by a separate commit to reduce number of unrelated changes. http://reviews.llvm.org/D13340 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits