[PowerPC] Correctly use ELFv2 ABI on all OS's that use the ELFv2 ABI

[brad]

Summary

I noticed D144232. This makes sure the ELFv2 ABI is utilized properly on all OS's that use the ELFv2 ABI.

[dim]

LGTM, some minor clang-format nits, but these aren't critical (to me at least :)

clang/lib/Basic/Targets/PPC.h

432

clang-format seems to want to format the if as:

if ((Triple.isOSFreeBSD() && (Triple.getOSVersion().empty() ||
                              Triple.getOSMajorVersion() >= 13)) ||
    Triple.isOSOpenBSD() || Triple.isMusl())

E,g. it groups the whole FreeBSD expression together. I didn't know it could do this, but it seems relatively clear.

llvm/lib/Target/PowerPC/PPCTargetMachine.cpp

242

similarly for this one, clang-format likes to format it as:

if ((TT.isOSFreeBSD() &&
     (TT.getOSVersion().empty() || TT.getOSMajorVersion() >= 13)) ||
    TT.isOSOpenBSD() || TT.isMusl())

[brad]

reformatting.

[brad]

clang/lib/Basic/Targets/PPC.h

432

clang-format seems to want to format the if as:

if ((Triple.isOSFreeBSD() && (Triple.getOSVersion().empty() ||
                              Triple.getOSMajorVersion() >= 13)) ||
    Triple.isOSOpenBSD() || Triple.isMusl())

E,g. it groups the whole FreeBSD expression together. I didn't know it could do this, but it seems relatively clear.

I can see both ways of doing things. I try to keep it as compact as possible. But I'll go with the suggested format.

[nemanjai]

clang/lib/Basic/Targets/PPC.h

430–436

I am generally not a fan of keeping multiple instances of a somewhat complex conditions in sync. I wonder if the community would be OK with one of:

• Adding a static function in Triple.h to test this (static bool isPPCELFv2ABI(const Triple &))
• Adding a member function Triple::isPPCELFv2ABI() (similar to Triple::isWatchABI())

Then of course, we can use that function in both places.

[brad]

clang/lib/Basic/Targets/PPC.h

430–436

I am generally not a fan of keeping multiple instances of a somewhat complex conditions in sync. I wonder if the community would be OK with one of:

• Adding a static function in Triple.h to test this (static bool isPPCELFv2ABI(const Triple &))
• Adding a member function Triple::isPPCELFv2ABI() (similar to Triple::isWatchABI())

Then of course, we can use that function in both places.

Ultimately something like that would be better as it's used in 3 different spots.

[dim]

Yeah, this looks quite a bit nicer, and should be more maintainable. Thanks.

[nemanjai]

Thanks for doing this. LGTM.

clang/lib/Basic/Targets/PPC.h

       LongDoubleWidth = 64;
       LongDoubleAlign = DoubleAlign = 32;
       LongDoubleFormat = &llvm::APFloat::IEEEdouble();
     } else if ((Triple.getArch() == llvm::Triple::ppc64le)) {
       DataLayout = "e-m:e-i64:64-n32:64";
       ABI = "elfv2";
     } else {
       DataLayout = "E-m:e-i64:64-n32:64";
-      ABI = "elfv1";
+      if ((Triple.isOSFreeBSD() && (Triple.getOSVersion().empty() ||
+          Triple.getOSMajorVersion() >= 13)) || Triple.isOSOpenBSD() ||
+          Triple.isMusl())
+        ABI = "elfv2";
+      else
+        ABI = "elfv1";
     }
 
     if (Triple.isOSFreeBSD() || Triple.isOSOpenBSD() || Triple.isMusl()) {
       LongDoubleWidth = LongDoubleAlign = 64;
       LongDoubleFormat = &llvm::APFloat::IEEEdouble();
     }
 
     if (Triple.isOSAIX() || Triple.isOSLinux())
       DataLayout += "-S128-v256:256:256-v512:512:512";

llvm/lib/Target/PowerPC/PPCTargetMachine.cpp

 
   assert(Options.MCOptions.getABIName().empty() &&
          "Unknown target-abi option!");
 
   switch (TT.getArch()) {
   case Triple::ppc64le:
     return PPCTargetMachine::PPC_ABI_ELFv2;
   case Triple::ppc64:
-    return PPCTargetMachine::PPC_ABI_ELFv1;
+    if ((TT.isOSFreeBSD() && (TT.getOSVersion().empty() ||
+      TT.getOSMajorVersion() >= 13)) || TT.isOSOpenBSD() || TT.isMusl())
+      return PPCTargetMachine::PPC_ABI_ELFv2;
+    else
+      return PPCTargetMachine::PPC_ABI_ELFv1;
   default:
     return PPCTargetMachine::PPC_ABI_UNKNOWN;
   }
 }
 
 static Reloc::Model getEffectiveRelocModel(const Triple &TT,
                                            std::optional<Reloc::Model> RM) {
   assert((!TT.isOSAIX() || !RM || *RM == Reloc::PIC_) &&

llvm/test/CodeGen/PowerPC/ppc64-elf-abi.ll

 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu < %s | FileCheck %s -check-prefix=CHECK-ELFv1
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu -target-abi elfv1 < %s | FileCheck %s -check-prefix=CHECK-ELFv1
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu -target-abi elfv2 < %s | FileCheck %s -check-prefix=CHECK-ELFv2
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu < %s | FileCheck %s -check-prefix=CHECK-ELFv2
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu -target-abi elfv1 < %s | FileCheck %s -check-prefix=CHECK-ELFv1
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu -target-abi elfv2 < %s | FileCheck %s -check-prefix=CHECK-ELFv2
 
-; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd < %s | FileCheck %s -check-prefix=CHECK-ELFv1
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-linux-musl < %s | FileCheck %s -check-prefix=CHECK-ELFv2
+
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd12 < %s | FileCheck %s -check-prefix=CHECK-ELFv1
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd < %s | FileCheck %s -check-prefix=CHECK-ELFv2
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd -target-abi elfv1 < %s | FileCheck %s -check-prefix=CHECK-ELFv1
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd -target-abi elfv2 < %s | FileCheck %s -check-prefix=CHECK-ELFv2
 
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-openbsd < %s | FileCheck %s -check-prefix=CHECK-ELFv2
+
 ; CHECK-ELFv2: .abiversion 2
 ; CHECK-ELFv1-NOT: .abiversion 2

llvm/test/CodeGen/PowerPC/pr47373.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc -mtriple=powerpc64-unknown-freebsd13.0 -verify-machineinstrs \
+; RUN: llc -mtriple=powerpc64-unknown-freebsd12.0 -verify-machineinstrs \
 ; RUN:   -mcpu=ppc64 -ppc-asm-full-reg-names < %s | FileCheck %s
 @a = local_unnamed_addr global ptr null, align 8
 
 ; Function Attrs: nounwind
 define void @d() local_unnamed_addr #0 {
 ; CHECK-LABEL: d:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    mflr r0