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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
