[PATCH] D144232: [PowerPC] Correctly use ELFv2 ABI on FreeBSD/powerpc64

[Alfredo Dal'Ava Júnior via Phabricator via llvm-commits]

adalava added a comment.

Nice catch! 
I'm not a LLVM maintainer but from FreeBSD PowerPC side I agree with the changes, only added two suggestions that could improve code readability



================
Comment at: clang/lib/Basic/Targets/PPC.h:428
       ABI = "elfv2";
+    } else if (Triple.isOSFreeBSD() && (Triple.getOSMajorVersion() == 0 ||
+                                        Triple.getOSMajorVersion() >= 13)) {
----------------
The comparison `Triple.getOSMajorVersion() == 0` works, but I think `Triple.getOSVersion().empty()`  would better describe the intent


================
Comment at: llvm/lib/Target/PowerPC/PPCTargetMachine.cpp:237
+  if (TT.isOSFreeBSD() && TT.getArch() == Triple::ppc64 &&
+      (TT.getOSMajorVersion() == 0 || TT.getOSMajorVersion() >= 13)) {
+    return PPCTargetMachine::PPC_ABI_ELFv2;
----------------
Same here, it could be `TT.getOSVersion().empty()` instead of `TT.getOSMajorVersion() == 0`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144232/new/

https://reviews.llvm.org/D144232