[clang][driver][NFC] Call IsARMBigEndain function only for isARM and isThumb.

[simpal01]

Summary

IsARMBIgEndian function returns true only if:

1. The triples are either arm or thumb and the commandline has the option -mbig-endian
2. The triples are either armeb or thumbeb.

Missing the checking of arm or thumb triples in the
first case pass through the --be8 endian flag to
linker For AArch64 as well which is not expected.
This is the regression happened from the previous
patch https://reviews.llvm.org/D154786.

It is better to refactor to only call IsARMBigEndian
for isARM and isthumb satisfying conditions which
keeps ARM and AArch64 separate.

[peter.smith]

clang/lib/Driver/ToolChains/Arch/ARM.cpp

39

Is this the right place to fix?

I would expect it to be a precondition that the Triple was Arm or Thumb before calling isARMBigEndian?

For example

if (Triple.isARM() || Triple.isThumb() || Triple.isAArch64()) {
    bool IsBigEndian = arm::isARMBigEndian(Triple, Args);
    if (IsBigEndian)
      arm::appendBE8LinkFlag(Args, CmdArgs, Triple);
    IsBigEndian = IsBigEndian || Arch == llvm::Triple::aarch64_be;
    CmdArgs.push_back(IsBigEndian ? "-EB" : "-EL");
  }

Shouldn't this be refactored to only call isARMBigEndian for isARM and isThumb? Something like:

if ((Triple.isARM() || Triple.isThumb()) {
  bool BigEndian = arm::isARMBigEndian(Triple, Args);
  if (BigEndian)
    arm::appendBE8LinkFlag(Args, CmdArgs, Triple);
  CmdArgs.push_back(IsBigEndian ? "-EB" : "-EL");
} else if (Triple.isAArch64)) {
  CmdArgs.push_back(Arch == llvm::Triple::aarch64_be ? "-EB" : "-EL");
}

This is a bit longer but it is easier to read and keeps ARM and AArch64 separate.

[simpal01]

Addressing review comments.

[MaskRay]

Sounds good to make arm::isARMBigEndian focus on AArch32 :)

[MaskRay]

Add NFC if this is NFC?

[simpal01]

In D155808#4528543, @MaskRay wrote:

Add NFC if this is NFC?

Done

clang/lib/Driver/ToolChains/Arch/ARM.cpp

bool arm::isARMMProfile(const llvm::Triple &Triple) {
   llvm::StringRef Arch = Triple.getArchName();
   return llvm::ARM::parseArchProfile(Arch) == llvm::ARM::ProfileKind::M;
 }
 
 // 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.
 bool arm::isARMBigEndian(const llvm::Triple &Triple, const ArgList &Args) {
+  if ((Triple.getArch() == llvm::Triple::arm ||
+       Triple.getArch() == llvm::Triple::thumb)) {
     if (Arg *A = Args.getLastArg(options::OPT_mlittle_endian,
                                  options::OPT_mbig_endian)) {
       return !A->getOption().matches(options::OPT_mlittle_endian);
     }
+  }
 
   return Triple.getArch() == llvm::Triple::armeb ||
          Triple.getArch() == llvm::Triple::thumbeb;
 }
 
 // True if A-profile.
 bool arm::isARMAProfile(const llvm::Triple &Triple) {
   llvm::StringRef Arch = Triple.getArchName();