[PATCH] D52784: [ARM][AArch64] Pass through endianness flags to the GNU assembler and linker

2018-10-22 Thread Peter Smith via Phabricator via cfe-commits 
peter.smith added a comment.

Added LLVM_FALLTHROUGH; in r344890.


Repository:
  rC Clang

https://reviews.llvm.org/D52784



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

[PATCH] D52784: [ARM][AArch64] Pass through endianness flags to the GNU assembler and linker

2018-10-19 Thread Dávid Bolvanský via Phabricator via cfe-commits 
xbolva00 added inline comments.



Comment at: lib/Driver/ToolChains/Gnu.cpp:241
+  case llvm::Triple::thumbeb:
+IsBigEndian = true;
+  case llvm::Triple::arm:

peter.smith wrote:
> xbolva00 wrote:
> > Gnu.cpp:241:17: warning: this statement may fall through 
> > [-Wimplicit-fallthrough=]
> >  IsBigEndian = true;
> The fall through is intentional in this case.
Please mark it with LLVM_FALLTHROUGH then.


Repository:
  rC Clang

https://reviews.llvm.org/D52784



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

[PATCH] D52784: [ARM][AArch64] Pass through endianness flags to the GNU assembler and linker

2018-10-19 Thread Peter Smith via Phabricator via cfe-commits 
peter.smith added a comment.

Thanks for pointing that out, I'm out of office today, will look at describing 
the intention to fall through when I get back in on Monday.




Comment at: lib/Driver/ToolChains/Gnu.cpp:241
+  case llvm::Triple::thumbeb:
+IsBigEndian = true;
+  case llvm::Triple::arm:

xbolva00 wrote:
> Gnu.cpp:241:17: warning: this statement may fall through 
> [-Wimplicit-fallthrough=]
>  IsBigEndian = true;
The fall through is intentional in this case.


Repository:
  rC Clang

https://reviews.llvm.org/D52784



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

[PATCH] D52784: [ARM][AArch64] Pass through endianness flags to the GNU assembler and linker

2018-10-18 Thread Dávid Bolvanský via Phabricator via cfe-commits 
xbolva00 added inline comments.



Comment at: lib/Driver/ToolChains/Gnu.cpp:241
+  case llvm::Triple::thumbeb:
+IsBigEndian = true;
+  case llvm::Triple::arm:

Gnu.cpp:241:17: warning: this statement may fall through 
[-Wimplicit-fallthrough=]
 IsBigEndian = true;


Repository:
  rC Clang

https://reviews.llvm.org/D52784



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

[PATCH] D52784: [ARM][AArch64] Pass through endianness flags to the GNU assembler and linker

2018-10-16 Thread Nick Desaulniers via Phabricator via cfe-commits 
nickdesaulniers added a comment.

Thanks again for this patch!


Repository:
  rC Clang

https://reviews.llvm.org/D52784



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

[PATCH] D52784: [ARM][AArch64] Pass through endianness flags to the GNU assembler and linker

2018-10-16 Thread Peter Smith via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rC344597: [ARM][AArch64] Pass through endian flags to 
assembler and linker. (authored by psmith, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D52784?vs=169689=169799#toc

Repository:
  rC Clang

https://reviews.llvm.org/D52784

Files:
  lib/Driver/ToolChains/Arch/ARM.cpp
  lib/Driver/ToolChains/Arch/ARM.h
  lib/Driver/ToolChains/Gnu.cpp
  lib/Driver/ToolChains/NetBSD.cpp
  test/Driver/linux-as.c
  test/Driver/linux-ld.c

Index: lib/Driver/ToolChains/Arch/ARM.h
===
--- lib/Driver/ToolChains/Arch/ARM.h
+++ lib/Driver/ToolChains/Arch/ARM.h
@@ -29,7 +29,7 @@
 StringRef getLLVMArchSuffixForARM(llvm::StringRef CPU, llvm::StringRef Arch,
   const llvm::Triple );
 
-void appendEBLinkFlags(const llvm::opt::ArgList ,
+void appendBE8LinkFlag(const llvm::opt::ArgList ,
llvm::opt::ArgStringList ,
const llvm::Triple );
 enum class ReadTPMode {
Index: lib/Driver/ToolChains/Arch/ARM.cpp
===
--- lib/Driver/ToolChains/Arch/ARM.cpp
+++ lib/Driver/ToolChains/Arch/ARM.cpp
@@ -643,7 +643,7 @@
   return llvm::ARM::getSubArch(ArchKind);
 }
 
-void arm::appendEBLinkFlags(const ArgList , ArgStringList ,
+void arm::appendBE8LinkFlag(const ArgList , ArgStringList ,
 const llvm::Triple ) {
   if (Args.hasArg(options::OPT_r))
 return;
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -228,6 +228,29 @@
   // The types are (hopefully) good enough.
 }
 
+// On Arm the endianness of the output file is determined by the target and
+// can be overridden by the pseudo-target flags '-mlittle-endian'/'-EL' and
+// '-mbig-endian'/'-EB'. Unlike other targets the flag does not result in a
+// normalized triple so we must handle the flag here.
+static bool isArmBigEndian(const llvm::Triple ,
+   const ArgList ) {
+  bool IsBigEndian = false;
+  switch (Triple.getArch()) {
+  case llvm::Triple::armeb:
+  case llvm::Triple::thumbeb:
+IsBigEndian = true;
+  case llvm::Triple::arm:
+  case llvm::Triple::thumb:
+if (Arg *A = Args.getLastArg(options::OPT_mlittle_endian,
+   options::OPT_mbig_endian))
+  IsBigEndian = !A->getOption().matches(options::OPT_mlittle_endian);
+break;
+  default:
+break;
+  }
+  return IsBigEndian;
+}
+
 static const char *getLDMOption(const llvm::Triple , const ArgList ) {
   switch (T.getArch()) {
   case llvm::Triple::x86:
@@ -240,10 +263,9 @@
 return "aarch64linuxb";
   case llvm::Triple::arm:
   case llvm::Triple::thumb:
-return "armelf_linux_eabi";
   case llvm::Triple::armeb:
   case llvm::Triple::thumbeb:
-return "armelfb_linux_eabi";
+return isArmBigEndian(T, Args) ? "armelfb_linux_eabi" : "armelf_linux_eabi";
   case llvm::Triple::ppc:
 return "elf32ppclinux";
   case llvm::Triple::ppc64:
@@ -337,8 +359,13 @@
   if (Args.hasArg(options::OPT_s))
 CmdArgs.push_back("-s");
 
-  if (Arch == llvm::Triple::armeb || Arch == llvm::Triple::thumbeb)
-arm::appendEBLinkFlags(Args, CmdArgs, Triple);
+  if (Triple.isARM() || Triple.isThumb() || Triple.isAArch64()) {
+bool IsBigEndian = isArmBigEndian(Triple, Args);
+if (IsBigEndian)
+  arm::appendBE8LinkFlag(Args, CmdArgs, Triple);
+IsBigEndian = IsBigEndian || Arch == llvm::Triple::aarch64_be;
+CmdArgs.push_back(IsBigEndian ? "-EB" : "-EL");
+  }
 
   // Most Android ARM64 targets should enable the linker fix for erratum
   // 843419. Only non-Cortex-A53 devices are allowed to skip this flag.
@@ -640,6 +667,7 @@
   case llvm::Triple::thumb:
   case llvm::Triple::thumbeb: {
 const llvm::Triple  = getToolChain().getTriple();
+CmdArgs.push_back(isArmBigEndian(Triple2, Args) ? "-EB" : "-EL");
 switch (Triple2.getSubArch()) {
 case llvm::Triple::ARMSubArch_v7:
   CmdArgs.push_back("-mfpu=neon");
@@ -672,6 +700,8 @@
   }
   case llvm::Triple::aarch64:
   case llvm::Triple::aarch64_be: {
+CmdArgs.push_back(
+getToolChain().getArch() == llvm::Triple::aarch64_be ? "-EB" : "-EL");
 Args.AddLastArg(CmdArgs, options::OPT_march_EQ);
 normalizeCPUNamesForAssembler(Args, CmdArgs);
 
Index: lib/Driver/ToolChains/NetBSD.cpp
===
--- lib/Driver/ToolChains/NetBSD.cpp
+++ lib/Driver/ToolChains/NetBSD.cpp
@@ -164,7 +164,7 @@
 break;
   case llvm::Triple::armeb:
   case llvm::Triple::thumbeb:
-arm::appendEBLinkFlags(Args, CmdArgs, ToolChain.getEffectiveTriple());
+arm::appendBE8LinkFlag(Args, CmdArgs, ToolChain.getEffectiveTriple());
 CmdArgs.push_back("-m");
 switch

[PATCH] D52784: [ARM][AArch64] Pass through endianness flags to the GNU assembler and linker

2018-10-15 Thread Nick Desaulniers via Phabricator via cfe-commits 
nickdesaulniers accepted this revision.
nickdesaulniers added a comment.
This revision is now accepted and ready to land.

Thanks for this patch.  With it I was able to link+boot a BE aarch64 Linux 
kernel (and a LE aarch64 Linux kernel).




Comment at: lib/Driver/ToolChains/Gnu.cpp:268
   case llvm::Triple::thumbeb:
-return "armelfb_linux_eabi";
+return (isArmBigEndian(T, Args)) ? "armelfb_linux_eabi"
+ : "armelf_linux_eabi";

probably don't need the parens around `isArmBigEndian(...)`.


https://reviews.llvm.org/D52784



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

[PATCH] D52784: [ARM][AArch64] Pass through endianness flags to the GNU assembler and linker

2018-10-15 Thread Peter Smith via Phabricator via cfe-commits 
peter.smith updated this revision to Diff 169689.
peter.smith marked 3 inline comments as done.
peter.smith added a comment.

Updated diff to reflect review comments. Main changes are:

- isArmBigEndian always returns false if the target architecture isn't Arm.
- Added tests to make sure "--be8" doesn't get added by mistake (would have 
been in previous patch for aarch64_be arch with -mbig-endian flag.


https://reviews.llvm.org/D52784

Files:
  lib/Driver/ToolChains/Arch/ARM.cpp
  lib/Driver/ToolChains/Arch/ARM.h
  lib/Driver/ToolChains/Gnu.cpp
  lib/Driver/ToolChains/NetBSD.cpp
  test/Driver/linux-as.c
  test/Driver/linux-ld.c

Index: test/Driver/linux-ld.c
===
--- test/Driver/linux-ld.c
+++ test/Driver/linux-ld.c
@@ -1759,6 +1759,7 @@
 // RUN:   | FileCheck --check-prefix=CHECK-ARMEB %s
 // CHECK-ARMEB: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
 // CHECK-ARMEB-NOT: "--be8"
+// CHECK-ARMEB: "-EB"
 // CHECK-ARMEB: "-m" "armelfb_linux_eabi"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
@@ -1768,16 +1769,88 @@
 // RUN:   | FileCheck --check-prefix=CHECK-ARMV7EB %s
 // CHECK-ARMV7EB: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
 // CHECK-ARMV7EB: "--be8"
+// CHECK-ARMV7EB: "-EB"
 // CHECK-ARMV7EB: "-m" "armelfb_linux_eabi"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=armv7-unknown-linux \
+// RUN: -mbig-endian \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-ARMV7EB %s
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=armebv7-unknown-linux \
+// RUN: -mbig-endian \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-ARMV7EB %s
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=armv7-unknown-linux \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-ARMV7EL %s
+// CHECK-ARMV7EL: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-ARMV7EL-NOT: "--be8"
+// CHECK-ARMV7EL: "-EL"
+// CHECK-ARMV7EL: "-m" "armelf_linux_eabi"
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=armebv7-unknown-linux \
+// RUN: -mlittle-endian \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-ARMV7EL %s
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=armv7-unknown-linux \
+// RUN: -mlittle-endian \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-ARMV7EL %s
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=aarch64_be-unknown-linux \
 // RUN: --gcc-toolchain="" \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-AARCH64BE %s
 // CHECK-AARCH64BE: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-AARCH64BE-NOT: "--be8"
+// CHECK-AARCH64BE: "-EB"
 // CHECK-AARCH64BE: "-m" "aarch64linuxb"
 
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=aarch64-unknown-linux \
+// RUN: -mbig-endian \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-AARCH64BE %s
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=aarch64_be-unknown-linux \
+// RUN: -mbig-endian \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-AARCH64BE %s
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=aarch64-unknown-linux \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-AARCH64LE %s
+// CHECK-AARCH64LE: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-AARCH64LE-NOT: "--be8"
+// CHECK-AARCH64LE: "-EL"
+// CHECK-AARCH64LE: "-m" "aarch64linux"
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=aarch64_be-unknown-linux \
+// RUN: -mlittle-endian \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-AARCH64LE %s
+
 // Check dynamic-linker for musl-libc
 // RUN: %clang %s -### -o %t.o 2>&1 \
 // RUN: --target=i386-pc-linux-musl \
Index: test/Driver/linux-as.c
===
--- test/Driver/linux-as.c
+++ test/Driver/linux-as.c
@@ -3,129 +3,160 @@
 // RUN: %clang -target arm-linux -### \
 // RUN:   -no-integrated-as -c %s 2>&1 \
 // RUN:   | FileCheck

[PATCH] D52784: [ARM][AArch64] Pass through endianness flags to the GNU assembler and linker

2018-10-15 Thread Peter Smith via Phabricator via cfe-commits 
peter.smith marked 7 inline comments as done.
peter.smith added a comment.

Thanks very much for the comments. I'll post an update shortly.




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";

nickdesaulniers wrote:
> nickdesaulniers wrote:
> > ```
> > bool IsBigEndian = isArmBigEndian(Triple, Args);
> > if (IsBigEndian)
> >   arm::appendBE8LinkFlag(Args, CmdArgs, Triple);
> > IsBigEndian |= Arch == llvm::Triple::aarch64_be;
> > CmdArgs.push_back(IsBigEndian ? "-EB" : "-EL");
> > ```
> `IsBigEndian |= Arch == llvm::Triple::aarch64_be;`
> 
> should be:
> 
> `IsBigEndian = IsBigEndian || Arch == llvm::Triple::aarch64_be;`
> 
> in order to not evaluate `Arch == llvm::Triple::aarch64_b` if `IsBigEndian` 
> is already true.
Thanks for the suggestion. One thing it highlighted was that isArmBigEndian 
could return true for an aarch64_be arch with -mbig-endian so I've rewritten 
isArmBigEndian to always return false if the architecture isn't Arm and have 
added some test cases to check that "--be8" doesn't sneak in.



Comment at: lib/Driver/ToolChains/Gnu.cpp:362
+}
+else if (Arch == llvm::Triple::aarch64_be)
+  EndianFlag = "-EB";

nickdesaulniers wrote:
> is having the `else if` on its own line what the formatter chose?
I'd forgot to run clang-format over that part of the code. I've adopted the 
snippet below which replaces it.



Comment at: lib/Driver/ToolChains/Gnu.cpp:703
   case llvm::Triple::aarch64_be: {
+if (getToolChain().getTriple().isLittleEndian())
+  CmdArgs.push_back("-EL");

nickdesaulniers wrote:
> 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?
I originally took that part from the Mips code, I've replaced it with a check 
against aarch64_be which is more consistent with the other Arm and AArch64 code.


https://reviews.llvm.org/D52784



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

[PATCH] D52784: [ARM][AArch64] Pass through endianness flags to the GNU assembler and linker

2018-10-12 Thread Nick Desaulniers via Phabricator via cfe-commits 
nickdesaulniers added inline comments.



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";

nickdesaulniers wrote:
> ```
> bool IsBigEndian = isArmBigEndian(Triple, Args);
> if (IsBigEndian)
>   arm::appendBE8LinkFlag(Args, CmdArgs, Triple);
> IsBigEndian |= Arch == llvm::Triple::aarch64_be;
> CmdArgs.push_back(IsBigEndian ? "-EB" : "-EL");
> ```
`IsBigEndian |= Arch == llvm::Triple::aarch64_be;`

should be:

`IsBigEndian = IsBigEndian || Arch == llvm::Triple::aarch64_be;`

in order to not evaluate `Arch == llvm::Triple::aarch64_b` if `IsBigEndian` is 
already true.


https://reviews.llvm.org/D52784



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

[PATCH] D52784: [ARM][AArch64] Pass through endianness flags to the GNU assembler and linker

2018-10-12 Thread Nick Desaulniers via Phabricator via cfe-commits 
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

[PATCH] D52784: [ARM][AArch64] Pass through endianness flags to the GNU assembler and linker

2018-10-12 Thread Peter Smith via Phabricator via cfe-commits 
peter.smith updated this revision to Diff 169386.
peter.smith added a comment.

I've decided to roll the linker changes in with the assembler ones as the 
linker use case affects the design. It turns out that only Arm needs to check 
to see if the -mbig-endian and -mlittle-endian flags override the triple as 
computeTargetTriple() accounts for them for AArch64.

I renamed arm::appendEBLinkFlags to arm::appendBE8LinkFlag as it exclusively 
adds "--be8" for Armv7 and above targets.


https://reviews.llvm.org/D52784

Files:
  lib/Driver/ToolChains/Arch/ARM.cpp
  lib/Driver/ToolChains/Arch/ARM.h
  lib/Driver/ToolChains/Gnu.cpp
  lib/Driver/ToolChains/NetBSD.cpp
  test/Driver/linux-as.c
  test/Driver/linux-ld.c

Index: test/Driver/linux-ld.c
===
--- test/Driver/linux-ld.c
+++ test/Driver/linux-ld.c
@@ -1759,6 +1759,7 @@
 // RUN:   | FileCheck --check-prefix=CHECK-ARMEB %s
 // CHECK-ARMEB: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
 // CHECK-ARMEB-NOT: "--be8"
+// CHECK-ARMEB: "-EB"
 // CHECK-ARMEB: "-m" "armelfb_linux_eabi"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
@@ -1768,16 +1769,64 @@
 // RUN:   | FileCheck --check-prefix=CHECK-ARMV7EB %s
 // CHECK-ARMV7EB: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
 // CHECK-ARMV7EB: "--be8"
+// CHECK-ARMV7EB: "-EB"
 // CHECK-ARMV7EB: "-m" "armelfb_linux_eabi"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=armv7-unknown-linux \
+// RUN: -mbig-endian \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-ARMV7EB %s
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=armv7-unknown-linux \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-ARMV7EL %s
+// CHECK-ARMV7EL: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-ARMV7EL: "-EL"
+// CHECK-ARMV7EL: "-m" "armelf_linux_eabi"
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=armebv7-unknown-linux \
+// RUN: -mlittle-endian \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-ARMV7EL %s
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=aarch64_be-unknown-linux \
 // RUN: --gcc-toolchain="" \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-AARCH64BE %s
 // CHECK-AARCH64BE: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-AARCH64BE: "-EB"
 // CHECK-AARCH64BE: "-m" "aarch64linuxb"
 
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=aarch64-unknown-linux \
+// RUN: -mbig-endian \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-AARCH64BE %s
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=aarch64-unknown-linux \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-AARCH64LE %s
+// CHECK-AARCH64LE: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-AARCH64LE: "-EL"
+// CHECK-AARCH64LE: "-m" "aarch64linux"
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=aarch64_be-unknown-linux \
+// RUN: -mlittle-endian \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-AARCH64LE %s
+
 // Check dynamic-linker for musl-libc
 // RUN: %clang %s -### -o %t.o 2>&1 \
 // RUN: --target=i386-pc-linux-musl \
Index: test/Driver/linux-as.c
===
--- test/Driver/linux-as.c
+++ test/Driver/linux-as.c
@@ -3,129 +3,160 @@
 // RUN: %clang -target arm-linux -### \
 // RUN:   -no-integrated-as -c %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-ARM %s
-// CHECK-ARM: as{{(.exe)?}}" "-mfloat-abi=soft"
+// CHECK-ARM: as{{(.exe)?}}" "-EL" "-mfloat-abi=soft"
 //
 // RUN: %clang -target arm-linux -mcpu=cortex-a8 -### \
 // RUN:   -no-integrated-as -c %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-ARM-MCPU %s
-// CHECK-ARM-MCPU: as{{(.exe)?}}" "-mfloat-abi=soft" "-mcpu=cortex-a8"
+// CHECK-ARM-MCPU: as{{(.exe)?}}" "-EL" "-mfloat-abi=soft" "-mcpu=cortex-a8"
 //
 // RUN: %clang -target arm-linux -mfpu=neon -### \
 // RUN:   -no-integrated-as -c %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-ARM-MFPU %s
-// CHECK-ARM-MFPU: as{{(.exe)?}}" "-mfloat-abi=soft" "-mfpu=neon"
+// CHECK-ARM-MFPU: as{{(.exe)?}}" "-EL" "-mfloat-abi=soft" "-mfpu=neon"
 //
 // RUN: %clang -target arm-linux -march=armv7-a -### \
 // RUN:   -no-integrated-as -c %s 2>&1 \
 // RUN:   | FileCheck