AArch64: Add command line option to select big or little endian

[cpirker]

Summary

Hi All,

I added -EL to select little endian and -EB to select big endian AArch64.
I also added -mlittle-endian as alias for -EL and -mbig-endian as alias for -EB.

Little endian:

--target=aarch64
--target=aarch64 -EL
--target=aarch64 -mlittle-endian
--target=aarch64_be -EL
--target=aarch64_be -mlittle-endian

Big endian:

--target=aarch64_be
--target=aarch64 -EB
--target=aarch64 -mbig-endian
--target=aarch64_be -EB
--target=aarch64_be -mbig-endian

Please review.

Thanks,
Christian

[t.p.northover]

Hi Chrisitan,

I'm not entirely convinced by the -EB/-EL version. It seems to be completely MIPS-specific in the GCC documentation, but -mbig-endian & -mlittle-endian apply to a few more (including ARM).

On the other hand, making it an alias does seem like rather a lot of sense implementation-wise. Perhaps make EB the alias for mbig--endian instead and drop the -EB tests? (On the grounds that it's not supported, but it's not really a big enough problem to bother with an error for).

Cheers.

Tim.

[cpirker]

Hi Tim,

these options are costumer driven.

Thanks,
Christian

[cpirker]

Hi Tim,

sorry, there is typo, I mean customer driven.

Thanks,
Christian

[t.p.northover]

Hi Christian,

sorry, there is typo, I mean customer driven.

I don't think that really changes anything. If the customers *want*
them, then they need to provide justification.

If they're already using them (or, say, GCC is adding them and we need
to for compatibility) then there should be some evidence of that.
Google didn't turn it up when I tried.

Cheers.

Tim.

[rnk]

Looks like -mbig-endian and -mlittle-endian are GCC options. We should
implement them for compatibility.
http://gcc.gnu.org/onlinedocs/gcc/ARM-Options.html

I don't think we should add -EL or -EB. They don't fit in with the set of
flags clang currently accepts.

[scott-0]

The -EB/-EL options already exist, but effect only the MIPS backend (they are silently ignored for ARM target). This patch introduces -mbig-endian/-mlittle-endian as aliases for them. The options are:

[this is really just a restatement of what Tim said earlier]

1. leave it as it is (possibly removing the tests for -EB/-EL from ARM targets)
2. invert the aliasing (but that would mean otherwise needless changes to the MIPS targets)
3. explicitly fault use of -EL/-EB for ARM targets (doesn't seem worthwhile)

Personally I vote for 1 with the -EB/-EL tests removed (or left in).

[atanasyan]

Support -EB/-EL is essential for MIPS. But I do not see any problems in inverting the aliasing. It looks like we need to change only a couple of lines in the computeTargetTriple() routine in that case.

[cpirker]

Hi All,

I made already a similar path for ARM targets (D3307).

As written by Scott, he would prefer it as it is, with the -EB/-EL tests removed.
I think we can leave these tests in.
So would it be accepted to commit this patch (with all test cases)?

Thanks,
Christian

[scott-0]

I'm also happy with inverting the aliasing; given the other comments, that seems to be the way to go.

[t.p.northover]

Hi Christian,

Yep. That sounds like a good idea.

[cpirker]

Hi,

I inverted the aliasing, and removed the -EL/-EB tests.
I made the same in D3307.

Thanks,
Christian

[atanasyan]

LGTM

[cpirker]

I committed this patch as r205966.

[cpirker]

Hi Simon,

I added you as reviewer.
Can you please "Accepted Revision" so that I can "Close Revision"?
As you can see in the comment above, the patch is already committed (rL205966).

Thanks,
Christian

[t.p.northover]

Hi Christian,

I added you as reviewer.
Can you please "Accepted Revision" so that I can "Close Revision"?
As you can see in the comment above, the patch is already committed.

You can accept and close the revision yourself if you want. Phab is
just a convenience to us: as long as the review takes place somehow we
don't really care about its state changes.

(Oh, and if you leave the comment box blank, no e-mail notification
gets sent. It's obviously up to you whether you want that in any given
situation).

Cheers.

Tim.

include/clang/Driver/Options.td

 def l : JoinedOrSeparate<["-"], "l">, Flags<[LinkerInput, RenderJoined]>;
 def lazy__framework : Separate<["-"], "lazy_framework">, Flags<[LinkerInput]>;
 def lazy__library : Separate<["-"], "lazy_library">, Flags<[LinkerInput]>;
-def EL : Flag<["-"], "EL">, Flags<[DriverOption]>;
-def EB : Flag<["-"], "EB">, Flags<[DriverOption]>;
+def mlittle_endian : Flag<["-"], "mlittle-endian">, Flags<[DriverOption]>;
+def EL : Flag<["-"], "EL">, Alias<mlittle_endian>;
+def mbig_endian : Flag<["-"], "mbig-endian">, Flags<[DriverOption]>;
+def EB : Flag<["-"], "EB">, Alias<mbig_endian>;
 def m16 : Flag<["-"], "m16">, Group<m_Group>, Flags<[DriverOption, CoreOption]>;
 def m32 : Flag<["-"], "m32">, Group<m_Group>, Flags<[DriverOption, CoreOption]>;
 def mqdsp6_compat : Flag<["-"], "mqdsp6-compat">, Group<m_Group>, Flags<[DriverOption,CC1Option]>,

lib/Driver/Driver.cpp

     }
   }
 
-  // Handle pseudo-target flags '-EL' and '-EB'.
-  if (Arg *A = Args.getLastArg(options::OPT_EL, options::OPT_EB)) {
-    if (A->getOption().matches(options::OPT_EL)) {
+  // Handle pseudo-target flags '-mlittle-endian'/'-EL' and
+  // '-mbig-endian'/'-EB'.
+  if (Arg *A = Args.getLastArg(options::OPT_mlittle_endian,
+                               options::OPT_mbig_endian)) {
+    if (A->getOption().matches(options::OPT_mlittle_endian)) {
       if (Target.getArch() == llvm::Triple::mips)
         Target.setArch(llvm::Triple::mipsel);
       else if (Target.getArch() == llvm::Triple::mips64)
         Target.setArch(llvm::Triple::mips64el);
+      else if (Target.getArch() == llvm::Triple::aarch64_be)
+        Target.setArch(llvm::Triple::aarch64);
     } else {
       if (Target.getArch() == llvm::Triple::mipsel)
         Target.setArch(llvm::Triple::mips);
       else if (Target.getArch() == llvm::Triple::mips64el)
         Target.setArch(llvm::Triple::mips64);
+      else if (Target.getArch() == llvm::Triple::aarch64)
+        Target.setArch(llvm::Triple::aarch64_be);
     }
   }
 

test/Driver/aarch64-cpus.c

 // Check target CPUs are correctly passed.
 
 // RUN: %clang -target aarch64 -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
+// RUN: %clang -target aarch64 -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
+// RUN: %clang -target aarch64_be -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
 // GENERIC: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"
 
 // RUN: %clang -target aarch64 -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53 %s
+// RUN: %clang -target aarch64 -mlittle-endian -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53 %s
+// RUN: %clang -target aarch64_be -mlittle-endian -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53 %s
 // CA53: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "cortex-a53"
 
 // RUN: %clang -target aarch64 -mcpu=cortex-a57 -### -c %s 2>&1 | FileCheck -check-prefix=CA57 %s
+// RUN: %clang -target aarch64 -mlittle-endian -mcpu=cortex-a57 -### -c %s 2>&1 | FileCheck -check-prefix=CA57 %s
+// RUN: %clang -target aarch64_be -mlittle-endian -mcpu=cortex-a57 -### -c %s 2>&1 | FileCheck -check-prefix=CA57 %s
 // CA57: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "cortex-a57"
 
 // RUN: %clang -target aarch64_be -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC-BE %s
+// RUN: %clang -target aarch64 -mbig-endian -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC-BE %s
+// RUN: %clang -target aarch64_be -mbig-endian -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC-BE %s
 // GENERIC-BE: "-cc1"{{.*}} "-triple" "aarch64_be{{.*}}" "-target-cpu" "generic"
 
 // RUN: %clang -target aarch64_be -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53-BE %s
+// RUN: %clang -target aarch64 -mbig-endian -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53-BE %s
+// RUN: %clang -target aarch64_be -mbig-endian -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53-BE %s
 // CA53-BE: "-cc1"{{.*}} "-triple" "aarch64_be{{.*}}" "-target-cpu" "cortex-a53"
 
 // RUN: %clang -target aarch64_be -mcpu=cortex-a57 -### -c %s 2>&1 | FileCheck -check-prefix=CA57-BE %s
+// RUN: %clang -target aarch64 -mbig-endian -mcpu=cortex-a57 -### -c %s 2>&1 | FileCheck -check-prefix=CA57-BE %s
+// RUN: %clang -target aarch64_be -mbig-endian -mcpu=cortex-a57 -### -c %s 2>&1 | FileCheck -check-prefix=CA57-BE %s
 // CA57-BE: "-cc1"{{.*}} "-triple" "aarch64_be{{.*}}" "-target-cpu" "cortex-a57"