[PATCH] D93138: Add initial support for multilibs in Baremetal toolchain.

[Zixuan Wu via Phabricator via cfe-commits]

zixuan-wu added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/BareMetal.cpp:177
   if (!getDriver().SysRoot.empty())
-    return getDriver().SysRoot;
+    return getDriver().SysRoot + SelectedMultilib.osSuffix();
 
----------------
abidh wrote:
> zixuan-wu wrote:
> > I think the multilib osSuffix added here will make every multilib contain include dir, and those include dirs are the same. For lib, there should be different multilib dir. 
> > 
> > So it's better to add osSuffix at construction function like following I think instead of be part of sysroot.
> > 
> > ```
> > if (!SysRoot.empty()) {
> >     llvm::sys::path::append(SysRoot, "lib", SelectedMultilib.osSuffix());
> >     getFilePaths().push_back(std::string(SysRoot));
> >   }
> > ```
> > 
> > @abidh 
> The multilib include directories are not necessarily the same.  In my opinion, having a separate include and lib directory for every multilib is much cleaner then having all multilib share an include directory and then you put multilib specific directories inside that.
So you are going to copy times of same include headers? For now,  GCC on RISCV is just sharing one same include directory and having multiple multilib directory under lib of root.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93138