jdoerfert added a comment.
Not a thorough review but comments to address.
================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1662
+ std::string archname = ArchName.str();
+ std::string gpuname = GPUArch.str();
+
----------------
Coding convention.
================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1674
+ std::string(LibraryPath + "/libdevice/lib" + libname + ".a"));
+ AOBFileNames.push_back(std::string(LibraryPath + "/lib" + libname + ".a"));
+
----------------
This will accumulate more and more entries in AOBFileNames, shouldn't you clear
it at the beginning?
================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1684
+
+ if (FoundAOB) {
+ std::string Err;
----------------
================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1810
+ for (std::string SDL_Name : SDL_Names) {
+ // THIS IS THE ONLY CALL TO SDLSearch
+ if (!(SDLSearch(D, DriverArgs, CC1Args, LibraryPaths, SDL_Name, ArchName,
----------------
================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1818
+ }
+}
+
----------------
Lots of places you create new std::strings for no reason, use StringRef or
const std::string & to avoid that (especially in range loops).
Please no conditional like `a != ".." && a != ".." ....`. Use a compile time
set or string switch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105191/new/
https://reviews.llvm.org/D105191
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
