tuliom marked 3 inline comments as done.
tuliom added inline comments.
================
Comment at: clang/lib/Driver/ToolChains/PPCLinux.cpp:74
+ const Driver &D,
+ const llvm::opt::ArgList &Args) const {
+ if (Args.hasArg(options::OPT_nostdlib, options::OPT_nostdlibxx))
----------------
nikic wrote:
> I don't think this formatting is right. You may find
> clang/tools/clang-format/clang-format-diff.py helpful.
>
> I use this script locally:
> ```
> #!/bin/sh
> git diff -U ${1:-HEAD} | clang/tools/clang-format/clang-format-diff.py -p1
> ```
> And then do something like `./clang_format_diff.sh | patch -p0` after
> checking that it did not reformat too much.
Thanks, this pointed out 2 errors.
================
Comment at: clang/lib/Driver/ToolChains/PPCLinux.cpp:96
bool HasUnsupportedCXXLib =
- StdLib == CST_Libcxx ||
+ (StdLib == CST_Libcxx && !defaultToIEEELongDouble()) ||
(StdLib == CST_Libstdcxx &&
----------------
qiucf wrote:
> `SupportIEEEFloat128` checks the library version:
>
> * If using libstdc++, then libstdc++ (GCC) >= 12.1.0 is okay.
> * If using libc++, then sorry, no libc++ supports `-mabi=ieeelongdouble` now.
> * Glibc should >= 2.32
>
> If the assumptions are still right, this changes its meaning (and
> `supportIBMLongDouble` has different meaning from it).
@qiucf In operating systems that switched to IEEE long double by default, all
libraries (and programs) are built with IEEE long double, including libc++.
================
Comment at: clang/test/Driver/lit.local.cfg:25
+
+if config.ppc_linux_default_ieeelongdouble == "ON":
+ config.available_features.add('ppc_linux_default_ieeelongdouble')
----------------
nikic wrote:
> I believe this isn't robust, because ON is not the only possible value.
> Instead, you'll want to canonicalize the variable:
> https://github.com/llvm/llvm-project/blob/03e6d9d9d1d48e43f3efc35eb75369b90d4510d5/clang/test/CMakeLists.txt#L4
Changed. This caused a few other modifications in the latest revision.
================
Comment at: clang/test/Driver/lit.local.cfg:26
+if config.ppc_linux_default_ieeelongdouble == "ON":
+ config.available_features.add('ppc_linux_default_ieeelongdouble')
----------------
qiucf wrote:
> Can we assume if we are compiling with `-mabi=ieeelongdouble`, then libc++
> 'must' be built with the same long double ABI? If I understand correctly,
> they're unrelated.
@qiucf I didn't understand this part. Are you suggesting to remove the long
double warnings because the way the compiler was built is unrelated to the way
libc++ was built?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D139450/new/
https://reviews.llvm.org/D139450
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
