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

Peter Smith via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 11 07:21:27 PDT 2018


peter.smith added a comment.

Thanks for the comments. I agree with you that the -EB and -EL flags need to be passed to the linker as well, I kind of expected that ld.bfd would infer it from the endianness of the first object but it doesn't seem to do that. If it's ok I'll do that in a separate patch?

I hope I've been able to explain the test. I'm on the fence about passing in the triple as a parameter, I have a mild preference for the way it is but if you'd like me to change it I can.



================
Comment at: lib/Driver/ToolChains/Gnu.cpp:543-549
+static const char* GetEndianArg(bool IsBigEndian, const ArgList &Args) {
+  if (Arg *A = Args.getLastArg(options::OPT_mlittle_endian,
+                               options::OPT_mbig_endian)) {
+      IsBigEndian = !A->getOption().matches(options::OPT_mlittle_endian);
+  }
+  return (IsBigEndian) ? "-EB" : "-EL";
+}
----------------
nickdesaulniers wrote:
> If you passed `llvm::Triple` instead of `bool`, then I think you could do something like:
> 
> ```
> static const char* GetEndianArg(const llvm::Triple &triple, const ArgList &Args) {
>   const bool IsBigEndian = triple == llvm::Triple::armeb ||
>                            triple == llvm::Triple::thumbeb ||
>                            triple == llvm::Triple::aarch64_be ||
>                            Args.getLastArg(options::OPT_mlittle_endian,
>                                            options::OPT_mbig_endian)->getOption().matches(options::OPT_mbig_endian);
>   return IsBigEndian ? "-EB" : "-EL";
> }
> ```
> 
> Might encapsulate the logic from the call sites better, but that's a minor nit.
I did think about separating it from the triple. In the end I thought it best to follow the existing switch on the triple and put up with a bit of duplication.

As it stands I think the above wouldn't quite work as we could have --target=armeb-linux-gnueabi -mlittle-endian which would get short-circuited to big endian if all the tests are done at once. 

I think it would be possible to do something like:

```
bool IsBigEndian = getTriple().getArch() == llvm::Triple::armeb ||
                                 getTriple().getArch() == llvm::Triple::thumbeb ||
                                 getTriple().getArch() == llvm::Triple::aarch64_be;
if (Arg *A = Args.getLastArg(options::OPT_mlittle_endian, options::OPT_mbig_endian))
      IsBigEndian = !A->getOption().matches(options::OPT_mlittle_endian);
return IsBigEndian ? "-EB" : "-EL"; 
```
but we'd only want to call it for Arm and AArch64 so I don't think it saves us much.




================
Comment at: lib/Driver/ToolChains/Gnu.cpp:544-547
+  if (Arg *A = Args.getLastArg(options::OPT_mlittle_endian,
+                               options::OPT_mbig_endian)) {
+      IsBigEndian = !A->getOption().matches(options::OPT_mlittle_endian);
+  }
----------------
nickdesaulniers wrote:
> Just so I understand this check, even if we didn't have a BE triple, we set `IsBigEndian` if `-mlittle-endian` was not set?  Is `-mlittle-endian` a default set flag, or does this set `"-EB"` even if no `-m*-endian` flag is specified (I think we want little-endian to be the default, and IIUC, this would change that)?
I stole the logic from ToolChain::ComputeLLVMTriple in ToolChain.cpp.

On entry to the function IsBigEndian will be set to true if the triple is one of armeb, thumbeb or aarch64eb. Otherwise it will be false.

The getLastArg(options::OPT_mlittle_endian, options_mbig_endian) will return NULL if neither was set, so we'd default to the value in the triple.




https://reviews.llvm.org/D52784





More information about the cfe-commits mailing list