nickdesaulniers added a comment.
Thanks for the addition of the flags to the linker. Interesting note about
`-m*-endian` only being applicable for armv7. Just some minor nits left.
With the current version of the patch, I can now assemble, link, and boot in a
virtualized environment a big-endian armv8 Linux kernel with Clang. :)
================
Comment at: lib/Driver/ToolChains/Gnu.cpp:231
+// On Arm and endianness of the output file is determined by the target and
+// can be overridden by the pseudo-target flags '-mlittle-endian'/'-EL' and
----------------
> // On Arm and endianness
drop first `and`
================
Comment at: lib/Driver/ToolChains/Gnu.cpp:261-264
+ if (isArmBigEndian(T, Args))
+ return "armelfb_linux_eabi";
+ else
+ return "armelf_linux_eabi";
----------------
would a `?:` ternary fit on one line here?
> return isArmBigEndian(T, Args) ? "armelfb_linux_eabi" : "armelf_linux_eabi";
================
Comment at: lib/Driver/ToolChains/Gnu.cpp:357-364
+ const char* EndianFlag = "-EL";
+ if (isArmBigEndian(Triple, Args)) {
+ EndianFlag = "-EB";
+ arm::appendBE8LinkFlag(Args, CmdArgs, Triple);
+ }
+ else if (Arch == llvm::Triple::aarch64_be)
+ EndianFlag = "-EB";
----------------
```
bool IsBigEndian = isArmBigEndian(Triple, Args);
if (IsBigEndian)
arm::appendBE8LinkFlag(Args, CmdArgs, Triple);
IsBigEndian |= Arch == llvm::Triple::aarch64_be;
CmdArgs.push_back(IsBigEndian ? "-EB" : "-EL");
```
================
Comment at: lib/Driver/ToolChains/Gnu.cpp:362
+ }
+ else if (Arch == llvm::Triple::aarch64_be)
+ EndianFlag = "-EB";
----------------
is having the `else if` on its own line what the formatter chose?
================
Comment at: lib/Driver/ToolChains/Gnu.cpp:667-670
+ if (isArmBigEndian(Triple2, Args))
+ CmdArgs.push_back("-EB");
+ else
+ CmdArgs.push_back("-EL");
----------------
Can we fit a ternary in one line here as well?
```
CmdArgs.push_back(isArmBigEndian(Triple2, Args) ? "-EB" : "-EL");
```
================
Comment at: lib/Driver/ToolChains/Gnu.cpp:703
case llvm::Triple::aarch64_be: {
+ if (getToolChain().getTriple().isLittleEndian())
+ CmdArgs.push_back("-EL");
----------------
earlier (L362), you check the endianess of the triple with:
```
Arch == llvm::Triple::aarch64_be
```
where `Arch` is `ToolChain.getArch()`.
I don't have a preference, but these two seem inconsistent. Can we either
check the explicit `llvm::Triple::` or call
`getToolChain().getTriple().isLittleEndian()` in both, rather than mix?
https://reviews.llvm.org/D52784
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
