[PATCH] D27662: [triple+llc+llvm-mc] Make triple and ABI name consistent

[Kovacsics Robert via Phabricator via llvm-commits]

KoviRobi added a comment.

Hello,

I'd be interested to see this merged as well, but when I run the tests after applying the patch (onto 0d7672e plus my patch on https://reviews.llvm.org/D21465) I find that the following tests fail:

- test/MC/Mips/macro-li.d.s
- test/MC/Mips/macro-li.s.s

I think it is to do with lib/Target/Mips/MCTargetDesc/MipsMCAsmInfo.cpp lines 29-36

  // FIXME: This condition isn't quite right but it's the best we can do until
  //        this object can identify the ABI. It will misbehave when using O32
  //        on a mips64*-* triple.
  if ((TheTriple.getArch() == Triple::mipsel) ||
      (TheTriple.getArch() == Triple::mips)) {
    PrivateGlobalPrefix = "$";
    PrivateLabelPrefix = "$";
  }

where we now should be able to access the ABI. But that file was not changed, the tests were broken by lib/Support/Triple.cpp at the end of getABIVariant:

  // We have to keep the Arch and Environment in sync at the moment
  // due to a false correlation between mips/mipsel and O32
  // and mips64/mips64el and N64 that is required by the MIPS backend.
  if (ABI == "o32")
    T = T.get32BitArchVariant();
  else if (ABI == "n32" || ABI == "n64")
    T = T.get64BitArchVariant();

as the failing tests, for n32 and n64, specify the mips architecture (though with -mcpu=mips64 which seems to get ignored) with n32 and n64 ABI. But the above change forces mips64, thus the labels change from "$tmp" to ".Ltmp". However, I am not sure which behaviour we want: use dollar prefix for O32 ABIs only, in which case we need to change the test and MipsMCAsmInfo.cpp, or use dollar prefix for mips (not mips64) targets?


Repository:
  rL LLVM

https://reviews.llvm.org/D27662