[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

[Zakk Chen via Phabricator via cfe-commits]

khchen added a comment.

I didn't check why I got error in  
Failed Tests (1):

  Clang :: Driver/riscv64-toolchain.c

Could you please double check it?
Thanks!



================
Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1599
+static std::string getGCCPath(const Driver &D, const ArgList &Args) {
+  llvm::StringRef GCCBasePath;
+  std::string GCCPath;
----------------
move into else


================
Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1604
+  const Arg *A = Args.getLastArg(clang::driver::options::OPT_gcc_toolchain);
+  if (A) {
+    GCCPath = findGCCPath(D, A->getValue());
----------------
if (const Arg *A = Args.getLastArg(clang::driver::options::OPT_gcc_toolchain))


================
Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1625
+                           const std::string &MultilibOutput,
+                           DetectedMultilibs &Result) {
+  llvm::StringSet<> AllABI;
----------------
bool -> static bool?
const std::string &MultilibOutput -> StringRef ?

The function name seem like it's not a target-specific function, but it's only support RISC-V now. Maybe we could rename it or make an assertion in function to check the target should be RISC-V?



================
Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1642
+  std::string CurrentArchOpt = Twine("march=", MArch).str();
+  std::string CurrentMCmodelOpt = llvm::StringSwitch<const char *>(CodeModel)
+                                      .Case("medium", "mcmodel=medany")
----------------
Could they be declared as StringRef and use llvm::StringSwitch<StringRef>?


================
Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1650
+
+  if (File) {
+    SmallVector<StringRef, 128> Lines;
----------------
```
if (!File)
  return;
```


================
Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1736
+
+  int rc = llvm::sys::ExecuteAndWait(GCCPath, GCCArgs, None, Redirects);
+
----------------
rc->RC


================
Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1744
+  // which multi-lib should be used.
+  return ScanGCCMultilibConfig(D, TargetTriple, Path, Args, MultilibOutput,
+                               Result);
----------------
ScanGCCMultilibConfig->scanGCCMultilibConfig


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97916/new/

https://reviews.llvm.org/D97916