dschuff accepted this revision. dschuff added inline comments. This revision is now accepted and ready to land.
================ Comment at: lib/Driver/ToolChain.cpp:318 + else + OSLibName = getOS(); llvm::sys::path::append(Path, "lib", OSLibName); ---------------- sbc100 wrote: > dschuff wrote: > > Is this logic intended to replace what was removed from CommonArgs.cpp? > > Should there be an assert here too? > Just didn't see the point of that assert. Can you see what the intention > might be? I don't see why AddRunTimeLibs() should be callable for any/all > triples, do you? I would have had to add llvm::Triple::Unknown to the list > of supported OSs, which seemed strange. I assume it was just because different OSes have different conventions for the rtlib, and if the OS is unknown and/or there's no particular support, then there might be a bug. But OTOH it doesn't seem bad to have some reasonable default either. For that matter it also seems a little weird that now we are letting the binary format be the determining thing for the Compiler-RT path. But I guess the real issue is that none of the OS, arch, or binary format is really the determining thing; it's really the toolchain/SDK or distribution of the compiler that determines what the path should be, and that can be affected by a lot of other things (e.g. is it the "system" compiler or not, is it a cross compiler, etc). So... yeah I think having this as a default makes as much sense as asserting, and when someone needs to add support for their distribution, then I suppose it's on them to refactor as needed and keep the tests working. https://reviews.llvm.org/D39218 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
