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

2023-02-27 Thread Brad Smith via Phabricator via cfe-commits 
brad added a comment.

In D144232#4155966 , @adalava wrote:

> I agree with D144321 , sorry for the late 
> reply.  @pkubaj and @brad , thanks for pushing it. I think D144232 
>  can be abandoned, right?

Correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144232

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2023-02-27 Thread Alfredo Dal'Ava Júnior via Phabricator via cfe-commits 
adalava added a comment.

In D144232#4137367 , @dim wrote:

> In D144232#4136787 , @brad wrote:
>
>> I noticed this review. I have provided a more complete diff for review at 
>> D144321 .
>
> Yeah I think that is probably the better option, hope @pkubaj and @adalava 
> agree with that?
>
> As Brad's version covers both FreeBSD and OpenBSD, and also updates a bunch 
> of unit tests, which this review appears to break (see the Unit Tests 
> https://reviews.llvm.org/harbormaster/unit/214420/).

I agree with D144321 , sorry for the late 
reply.  @pkubaj and @brad , thanks for pushing it. I think D144232 
 can be abandoned, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144232

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2023-02-18 Thread Dimitry Andric via Phabricator via cfe-commits 
dim added a comment.

In D144232#4136787 , @brad wrote:

> I noticed this review. I have provided a more complete diff for review at 
> D144321 .

Yeah I think that is probably the better option, hope @pkubaj and @adalava 
agree with that?

As Brad's version covers both FreeBSD and OpenBSD, and also updates a bunch of 
unit tests, which this review appears to break (see the Unit Tests 
https://reviews.llvm.org/harbormaster/unit/214420/).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144232

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2023-02-17 Thread Brad Smith via Phabricator via cfe-commits 
brad added a comment.

I noticed this review. I have provided a more complete diff for review at 
D144321 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144232

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2023-02-17 Thread Piotr Kubaj via Phabricator via cfe-commits 
pkubaj updated this revision to Diff 498378.
pkubaj added a comment.

Addres adalava's review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144232

Files:
  clang/lib/Basic/Targets/PPC.h
  llvm/lib/Target/PowerPC/PPCTargetMachine.cpp


Index: llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
===
--- llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
+++ llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
@@ -233,6 +233,11 @@
   assert(Options.MCOptions.getABIName().empty() &&
  "Unknown target-abi option!");
 
+  if (TT.isOSFreeBSD() && TT.getArch() == Triple::ppc64 &&
+  (TT.getOSVersion().empty() || TT.getOSMajorVersion() >= 13)) {
+return PPCTargetMachine::PPC_ABI_ELFv2;
+  }
+
   switch (TT.getArch()) {
   case Triple::ppc64le:
 return PPCTargetMachine::PPC_ABI_ELFv2;
Index: clang/lib/Basic/Targets/PPC.h
===
--- clang/lib/Basic/Targets/PPC.h
+++ clang/lib/Basic/Targets/PPC.h
@@ -425,6 +425,10 @@
 } else if ((Triple.getArch() == llvm::Triple::ppc64le)) {
   DataLayout = "e-m:e-i64:64-n32:64";
   ABI = "elfv2";
+} else if (Triple.isOSFreeBSD() && (Triple.getOSVersion().empty() ||
+Triple.getOSMajorVersion() >= 13)) {
+  DataLayout = "E-m:e-i64:64-n32:64";
+  ABI = "elfv2";
 } else {
   DataLayout = "E-m:e-i64:64-n32:64";
   ABI = "elfv1";


Index: llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
===
--- llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
+++ llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
@@ -233,6 +233,11 @@
   assert(Options.MCOptions.getABIName().empty() &&
  "Unknown target-abi option!");
 
+  if (TT.isOSFreeBSD() && TT.getArch() == Triple::ppc64 &&
+  (TT.getOSVersion().empty() || TT.getOSMajorVersion() >= 13)) {
+return PPCTargetMachine::PPC_ABI_ELFv2;
+  }
+
   switch (TT.getArch()) {
   case Triple::ppc64le:
 return PPCTargetMachine::PPC_ABI_ELFv2;
Index: clang/lib/Basic/Targets/PPC.h
===
--- clang/lib/Basic/Targets/PPC.h
+++ clang/lib/Basic/Targets/PPC.h
@@ -425,6 +425,10 @@
 } else if ((Triple.getArch() == llvm::Triple::ppc64le)) {
   DataLayout = "e-m:e-i64:64-n32:64";
   ABI = "elfv2";
+} else if (Triple.isOSFreeBSD() && (Triple.getOSVersion().empty() ||
+Triple.getOSMajorVersion() >= 13)) {
+  DataLayout = "E-m:e-i64:64-n32:64";
+  ABI = "elfv2";
 } else {
   DataLayout = "E-m:e-i64:64-n32:64";
   ABI = "elfv1";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2023-02-16 Thread Alfredo Dal'Ava Júnior via Phabricator via cfe-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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2023-02-16 Thread Piotr Kubaj via Phabricator via cfe-commits 
pkubaj updated this revision to Diff 498185.
pkubaj added a comment.

Fix indenting according to clang-format suggestions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144232

Files:
  clang/lib/Basic/Targets/PPC.h
  llvm/lib/Target/PowerPC/PPCTargetMachine.cpp


Index: llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
===
--- llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
+++ llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
@@ -233,6 +233,11 @@
   assert(Options.MCOptions.getABIName().empty() &&
  "Unknown target-abi option!");
 
+  if (TT.isOSFreeBSD() && TT.getArch() == Triple::ppc64 &&
+  (TT.getOSMajorVersion() == 0 || TT.getOSMajorVersion() >= 13)) {
+return PPCTargetMachine::PPC_ABI_ELFv2;
+  }
+
   switch (TT.getArch()) {
   case Triple::ppc64le:
 return PPCTargetMachine::PPC_ABI_ELFv2;
Index: clang/lib/Basic/Targets/PPC.h
===
--- clang/lib/Basic/Targets/PPC.h
+++ clang/lib/Basic/Targets/PPC.h
@@ -425,6 +425,10 @@
 } else if ((Triple.getArch() == llvm::Triple::ppc64le)) {
   DataLayout = "e-m:e-i64:64-n32:64";
   ABI = "elfv2";
+} else if (Triple.isOSFreeBSD() && (Triple.getOSMajorVersion() == 0 ||
+Triple.getOSMajorVersion() >= 13)) {
+  DataLayout = "E-m:e-i64:64-n32:64";
+  ABI = "elfv2";
 } else {
   DataLayout = "E-m:e-i64:64-n32:64";
   ABI = "elfv1";


Index: llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
===
--- llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
+++ llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
@@ -233,6 +233,11 @@
   assert(Options.MCOptions.getABIName().empty() &&
  "Unknown target-abi option!");
 
+  if (TT.isOSFreeBSD() && TT.getArch() == Triple::ppc64 &&
+  (TT.getOSMajorVersion() == 0 || TT.getOSMajorVersion() >= 13)) {
+return PPCTargetMachine::PPC_ABI_ELFv2;
+  }
+
   switch (TT.getArch()) {
   case Triple::ppc64le:
 return PPCTargetMachine::PPC_ABI_ELFv2;
Index: clang/lib/Basic/Targets/PPC.h
===
--- clang/lib/Basic/Targets/PPC.h
+++ clang/lib/Basic/Targets/PPC.h
@@ -425,6 +425,10 @@
 } else if ((Triple.getArch() == llvm::Triple::ppc64le)) {
   DataLayout = "e-m:e-i64:64-n32:64";
   ABI = "elfv2";
+} else if (Triple.isOSFreeBSD() && (Triple.getOSMajorVersion() == 0 ||
+Triple.getOSMajorVersion() >= 13)) {
+  DataLayout = "E-m:e-i64:64-n32:64";
+  ABI = "elfv2";
 } else {
   DataLayout = "E-m:e-i64:64-n32:64";
   ABI = "elfv1";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2023-02-16 Thread Piotr Kubaj via Phabricator via cfe-commits 
pkubaj created this revision.
pkubaj added reviewers: dim, nemanjai, adalava.
Herald added subscribers: shchenz, kbarton, hiraditya.
Herald added a project: All.
pkubaj requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Currently ELFv2 is mostly set, but some places still default to ELFv1.
While this mostly works, it breaks in particular when lld is used with LTO,
as reported in https://github.com/llvm/llvm-project/issues/46697.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144232

Files:
  clang/lib/Basic/Targets/PPC.h
  llvm/lib/Target/PowerPC/PPCTargetMachine.cpp


Index: llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
===
--- llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
+++ llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
@@ -233,6 +233,10 @@
   assert(Options.MCOptions.getABIName().empty() &&
  "Unknown target-abi option!");
 
+  if (TT.isOSFreeBSD() && TT.getArch() == Triple::ppc64 && 
(TT.getOSMajorVersion() == 0 || TT.getOSMajorVersion() >= 13)) {
+return PPCTargetMachine::PPC_ABI_ELFv2;
+  }
+
   switch (TT.getArch()) {
   case Triple::ppc64le:
 return PPCTargetMachine::PPC_ABI_ELFv2;
Index: clang/lib/Basic/Targets/PPC.h
===
--- clang/lib/Basic/Targets/PPC.h
+++ clang/lib/Basic/Targets/PPC.h
@@ -425,6 +425,9 @@
 } else if ((Triple.getArch() == llvm::Triple::ppc64le)) {
   DataLayout = "e-m:e-i64:64-n32:64";
   ABI = "elfv2";
+} else if (Triple.isOSFreeBSD() && (Triple.getOSMajorVersion() == 0 || 
Triple.getOSMajorVersion() >= 13)) {
+  DataLayout = "E-m:e-i64:64-n32:64";
+  ABI = "elfv2";
 } else {
   DataLayout = "E-m:e-i64:64-n32:64";
   ABI = "elfv1";


Index: llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
===
--- llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
+++ llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
@@ -233,6 +233,10 @@
   assert(Options.MCOptions.getABIName().empty() &&
  "Unknown target-abi option!");
 
+  if (TT.isOSFreeBSD() && TT.getArch() == Triple::ppc64 && (TT.getOSMajorVersion() == 0 || TT.getOSMajorVersion() >= 13)) {
+return PPCTargetMachine::PPC_ABI_ELFv2;
+  }
+
   switch (TT.getArch()) {
   case Triple::ppc64le:
 return PPCTargetMachine::PPC_ABI_ELFv2;
Index: clang/lib/Basic/Targets/PPC.h
===
--- clang/lib/Basic/Targets/PPC.h
+++ clang/lib/Basic/Targets/PPC.h
@@ -425,6 +425,9 @@
 } else if ((Triple.getArch() == llvm::Triple::ppc64le)) {
   DataLayout = "e-m:e-i64:64-n32:64";
   ABI = "elfv2";
+} else if (Triple.isOSFreeBSD() && (Triple.getOSMajorVersion() == 0 || Triple.getOSMajorVersion() >= 13)) {
+  DataLayout = "E-m:e-i64:64-n32:64";
+  ABI = "elfv2";
 } else {
   DataLayout = "E-m:e-i64:64-n32:64";
   ABI = "elfv1";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits