Page MenuHomeFreeBSD

[PowerPC64] backport of LLVM fixes in preparation for building FreeBSD/PowerPC64
AbandonedPublic

Authored by alfredo.junior_eldorado.org.br on May 21 2019, 7:30 PM.

Details

Summary

This is a fix that allows FreeBSD/PowerPC64 use LLVM as the main compiler/linker. The commit list bellow refers to a hash found at LLVM Project repo https://github.com/llvm/llvm-project/tree/master, currently tagged as LLVM 9.X. The changes were backported to 8.x

LLVM commit list:

From 44266b9e115ad172b1f6a88d15d4e7579812c0fc Mon Sep 17 00:00:00 2001
From: Martin Storsjo <martin@martin.st>
Date: Thu, 16 May 2019 06:49:13 +0000
Subject: [PATCH] [PPC64][libunwind] Fix r2 not properly restored

This change makes each unwind step inspect the instruction at the
return address and, if needed, read r2 from its saved location and
modify the context appropriately.

The unwind logic is able to handle both ELFv1 and ELFv2 stacks.

Reported by Bug 41050

Patch by Leandro Lupori!

Revision: https://reviews.llvm.org/D59694

llvm-svn: 360861

From 513d3658e708e38d2d4612fe59c7a3c3f64e8a5d
Author: Fangrui Song <maskray@google.com>
Date: Wed Apr 24 14:03:30 2019 +0000

[PPC64] Consider localentry offset when computing branch distance

Summary:
We don't take localentry offset into account, and thus may fail to
create a long branch when the gap is just a few bytes smaller than 2^25.

relocation R_PPC64_REL24 out of range: 33554432 is not in [-33554432, 33554431]
relocation R_PPC64_REL24 out of range: 33554436 is not in [-33554432, 33554431]

Fix that by adding the offset to the symbol VA.

Differential Revision: https://reviews.llvm.org/D61058

llvm-svn: 359094

Diff Detail

Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 25090
Build 23795: arc lint + arc unit

Event Timeline

alfredo.junior_eldorado.org.br planned changes to this revision.May 21 2019, 7:32 PM
alfredo.junior_eldorado.org.br retitled this revision from This is a batch of fixes that allows FreeBSD/PowerPC64 use LLVM as the main compiler/linker. The commit list bellow refers to hashs found at LLVM Project repo https://github.com/llvm/llvm-project/tree/master, currently tagged as LLVM 9.X. The... to [PowerPC64] backport of LLVM fixes in preparation for building FreeBSD/PowerPC64.
alfredo.junior_eldorado.org.br edited the summary of this revision. (Show Details)
alfredo.junior_eldorado.org.br edited the test plan for this revision. (Show Details)

I'm suspending the review until I can test on x86_64

add patch [PowerPC64] adds ABI parsing when specified on target triple

alfredo.junior_eldorado.org.br edited the test plan for this revision. (Show Details)

This is now ready for review.

Thanks!

emaste added a subscriber: emaste.May 22 2019, 6:58 PM

Do we currently have changes relative to upstream in any of the files affected by this review? (Knowing there are none, or what they are, may be useful in the future when we import 9.0.)

bdragon requested changes to this revision.Jun 12 2019, 10:07 PM

needs reroll, the backport of most of this landed with r349004.

This revision now requires changes to proceed.Jun 12 2019, 10:07 PM
alfredo.junior_eldorado.org.br edited the summary of this revision. (Show Details)
alfredo.junior_eldorado.org.br edited the test plan for this revision. (Show Details)

Since LLVM was upgraded to 8.0.1-rc2 and it already contains almost all of our changes, this review was updated with remaining required changes

dim accepted this revision.Jun 13 2019, 7:38 PM

LGTM (though I don't have the possibility of testing this at runtime, sorry :) ). Do we want to MFC this?

In D20337#445850, @dim wrote:

LGTM (though I don't have the possibility of testing this at runtime, sorry :) ). Do we want to MFC this?

@dim , thank you for updating the base LLVM.

I'm not sure MFC is necessary as previous FreeBSD/powerpc64 versions are using GCC's unwind code and this is not useful for other platforms when cross compiling to powerpc64.
Do you agree, @luporl, @bdragon ?

No MFC is necessary for this. FreeBSD 13.x is the switchover to LLVM-all-the-things on powerpc, so we'll stick with that.

@dim, since you're the keeper of LLVM, do you want to do the honors of committing this, or should I?

dim added a comment.Jun 26 2019, 4:11 PM

No MFC is necessary for this. FreeBSD 13.x is the switchover to LLVM-all-the-things on powerpc, so we'll stick with that.

I tend to MFC as much of the LLVM changes as possible, to ensure the diff between LLVM versions in 11.x, 12.x and 13.x is as minimal as possible. If this change would not cause any trouble in 11.x and 12.x, I will MFC them at some point.

@dim, since you're the keeper of LLVM, do you want to do the honors of committing this, or should I?

Please feel free to commit, thanks! :)

@dim, @jhibbits: this libunwind fix and https://bugs.llvm.org/show_bug.cgi?id=42332 bugfix we also need were just accepted for the LLVM 8.0.1. They are already in release/8.x branch.

Maybe you want update whole LLVM in base instead?

dim added a comment.Jun 26 2019, 7:35 PM

@dim, @jhibbits: this libunwind fix and https://bugs.llvm.org/show_bug.cgi?id=42332 bugfix we also need were just accepted for the LLVM 8.0.1. They are already in release/8.x branch.
Maybe you want update whole LLVM in base instead?

That's my plan anyway, I was just waiting for the final 8.0.1 tag to be placed. When that is done, I will import it ASAP.

alfredo.junior_eldorado.org.br edited the summary of this revision. (Show Details)

Update LLVM patches

jhibbits accepted this revision.Jun 27 2019, 7:10 PM

This can now be abandoned, 8.0.1-rc3 was pulled in just the other day.

Base LLVM was updated to 8.0.1-rc3 (da9211c814dfde69561ee93a0627bd056492b12e), so this can be abandoned

dim added inline comments.Jul 8 2019, 6:45 PM
contrib/libunwind/src/assembly.h
122

Hm, I do NOT see this particular change in rS349793. Apparently the DEFINE_LIBUNWIND_PRIVATE_FUNCTION macro went away in https://github.com/llvm/llvm-project/commit/e369a989fc37248f4aa05ab2d184270ac27b22e1#diff-99a7b5f768d8715507629783345d6010, and the upstream merge to 8.0.1 missed it?

How important are the added PPC64_OPD1/2 lines before and after the SYMBOL_NAME line?

@dim good catch. I talked to @luporl (who did the fix) and he confirmed this part was lost during backport from 9.0.0 to 8.0.1. I'll report upstream.

this part is related to ELFv1 binaries using libunwind. The ELFv2 environment is not prepared to run ELFv1 binaries, so I don't think we really need this change now, we can wait for upstream to fix.

@jhibbits, do you agree?