peter.smith added a comment.

I think that you may be better off posting a RFC to llvm-dev to get discussion 
on the best approach here. It would also be good to step back a bit and 
consider the requirements, as it stands it looks like this might be a solution 
for just one particular multilib layout and we might be able to get some input 
on some other use cases that others may have. I personally would like to enable 
clang to be able to query a gcc installation to find out (and potentially 
cache) its multlib directories as I've been told for the Arm embedded toolchain 
that the directory structure is not fixed and may change at any point in the 
future.

Another unrelated thing that may be worth doing is to check for other places 
where the target might affect the code-generation independently of -mfloat-abi, 
I'm thinking in particular of builtin functions as this has changed a bit over 
the past few years.



================
Comment at: lib/Driver/ToolChains/Gnu.cpp:1441
 
+static bool findArmEABIMultilibs(const Driver &D,
+                                 const llvm::Triple &TargetTriple,
----------------
When I saw EABIMultilibs I first thought of multilib support for arm-none-eabi 
, I think that this may be worth renaming to something like ArmLinuxGNUEABI as 
this could be specific to Linux Distributions. 


================
Comment at: lib/Driver/ToolChains/Gnu.cpp:1451
+  bool DefaultHardFloat = TargetTriple.isHardFloatEABI();
+  llvm::Triple AltTriple(DefaultHardFloat ?
+                             TargetTriple.getSoftFloatArchVariant() :
----------------
AltTriple doesn't seem to be used anywhere in the rest of the function?


================
Comment at: lib/Driver/ToolChains/Gnu.cpp:1454
+                             TargetTriple.getHardFloatArchVariant());
+  // We assume the Debian libdir/includedir arrangement for armel/armhf,
+  // since Debian and its descendents are apparently the only common Linux
----------------
Another place where this is used, and clang can't yet handle, is the GCC 
Embedded toolchain arm-none-eabi which includes both hard and soft float abi 
libraries. These have a different structure to the one presented here though.   


================
Comment at: lib/Driver/ToolChains/Gnu.cpp:1458
+  // SuSE and RedHat seem to stick with hard-float only and no libdir suffix.
+  // TODO: this (and a lot of other code here) could be generalized via the
+  // output of "gcc <flags> --print-multi-os-dir".
----------------
I definitely agree here. It seems to me to be quite fragile to hard-code in 
directory structures of gcc toolchains or linux distributions that can, and do, 
vary over time.


Repository:
  rC Clang

https://reviews.llvm.org/D52705



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to