This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC64] adds ABI parsing when specified on target triple
ClosedPublic

Authored by adalava on May 15 2019, 8:33 AM.

Details

Summary

LLVM supports OpenPower ABI ELFv2 when argument "-mabi=elfv2" if used on clang, as default ABI is ELFv1. FreeBSD support for PowerPC64 ELFv2 ABI with LLVM is in progress[1], so this patch adds an alternative way to specify ELFv2 ABI on target triple [2].

The following results are expected:

ELFv1 when using:
-target powerpc64-unknown-freebsd12.0
-target powerpc64-unknown-freebsd12.0 -mabi=elfv1
-target powerpc64-unknown-freebsd12.0-elfv1

ELFv2 when using:
-target powerpc64-unknown-freebsd12.0 -mabi=elfv2
-target powerpc64-unknown-freebsd12.0-elfv2

[1] https://wiki.freebsd.org/powerpc/llvm-elfv2
[2] https://clang.llvm.org/docs/CrossCompilation.html

Diff Detail

Repository
rL LLVM

Event Timeline

adalava created this revision.May 15 2019, 8:33 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMay 15 2019, 8:33 AM
adalava retitled this revision from [PowerPC64] recognize target triplet with ABI info (i.e. powerpc64-unknown-freebsd13-elfv2) to [PowerPC64] adds ABI parsing when specified on target triple.May 15 2019, 8:47 AM
adalava edited the summary of this revision. (Show Details)
adalava added reviewers: sfertile, emaste, MaskRay.
adalava added subscribers: luporl, Bdragon28.
MaskRay added inline comments.May 15 2019, 8:57 AM
clang/lib/Basic/Targets/PPC.h
382 ↗(On Diff #199620)

Have you researched how GNU as chooses the default ABI?

I think the change may break some Linux ppc64le users as they expect the default elfv2.

MaskRay accepted this revision.May 15 2019, 8:59 AM
MaskRay added inline comments.
clang/lib/Basic/Targets/PPC.h
382 ↗(On Diff #199620)

Sorry, it won't :) Then I think it is fine.

This revision is now accepted and ready to land.May 15 2019, 8:59 AM
MaskRay added inline comments.May 15 2019, 9:01 AM
llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
217 ↗(On Diff #199620)

The initial word should be capitalized and the sentence should end with a period. At least this part of the comment (if ELFv2 on target triple ) is redundant. I don't know if the other half powerpc64-unknown-freebsd12.0-elfv2 is useful...

adalava marked an inline comment as done.May 15 2019, 10:09 AM
adalava added inline comments.
clang/lib/Basic/Targets/PPC.h
382 ↗(On Diff #199620)

I didn't look the code, but behavior on GNU is the same as LLVM too:

  • powerpc64-* and elfv2 if "powerpc64le-*" and last

I found setting the ABI value here is useless so I decided to remove to keep it consistent with it's "sister" X86 class and checked that no tests are broken after this, but would be nice if someone with more LLVm experience could give another look.

adalava updated this revision to Diff 199633.May 15 2019, 10:20 AM
  • remove redundant comment
adalava marked 3 inline comments as done.May 15 2019, 10:22 AM
adalava added inline comments.
llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
217 ↗(On Diff #199620)

yeah, sounds like code clears explains itself. I'm removing the comment.

Will FreeBSD 13 or future releases support ELFv1? If not, it may be cleaner to not invent -elfv2 -elfv1 triples, but rather dispatch on the major version, e.g. powerpc64-unknown-freebsd13.0 could mean ELFv2.

adalava marked an inline comment as done.EditedMay 16 2019, 6:28 AM

Will FreeBSD 13 or future releases support ELFv1? If not, it may be cleaner to not invent -elfv2 -elfv1 triples, but rather dispatch on the major version, e.g. powerpc64-unknown-freebsd13.0 could mean ELFv2.

Invent -elfv2 -elfv1 and make it used by default is not the goal here but add ABI target triple support. Discussions at llvm IRC channel shown that provide ABI in target triple is more acceptable than using -mabi=elfv2. This patch is for when you want force an ABI other than default one (that was based on <arch>, <os> or <os_version>)

For FreeBSD, yes, the default ABI will be decided based on OS version on triple powerpc64-unknown-freebsd<os_version>. However, there's no agreement on what will be that <os_version>. Even if it's decided that RELEASE/13.0 will be ELFv2, CURRENT/Development/13.0 is ELFv1, LLVM can't assume 13.0 is ELFv2 at this moment or it will break users.

So, during the transition period, developers could use TARGET_ABI variable to build the FreeBSD PPC64 ELFv2 version. As example, building FreeBSD with make TARGET=powerpc TARGET_ARCH=powerpc64 TARGET_ABI=elfv2, where it's expected to form a target triple like powerpc64-unknown-freebsd13.0-elfv2, based on "<arch><sub>-<vendor>-<sys>-<abi>" definition. That is passed right away to clang.

If it's not possible, we would have to patch FreeBSD makefiles to not use TARGET_ABI variable value when TARGET_ARCH is powerpc64, and append -mabi=elfv2 instead. This sounds feasible, however clang running on final ELFv2 userland won't know it's on ELFv2 and users will need to specify -mabi=elfv2 and CFLAGS/CXXFLAGS to compile applications. This is unfeasible, specially for FreeBSD ports, as many packages didn't handle these variables correctly.

MaskRay accepted this revision.May 16 2019, 7:44 AM

Thanks for the explanation!

Reordered the code a bit and committed for you...

This revision was automatically updated to reflect the committed changes.

@adalava You need to run both check-llvm and check-clang, but it was my fault that I hadn't done this before committing...
Two clang tests failed (http://lab.llvm.org:8011/builders/clang-atom-d525-fedora-rel/builds/24521) It should have been fixed by rC361365