In D71600#1868284, @efriedma wrote:

On master, atomic.c is not built by default. It's only built if you explicitly request it with something like the CMake flag -DCOMPILER_RT_EXCLUDE_ATOMIC_BUILTIN=Off . If you're not doing that, any change to atomic.c has no effect.

In D71600#1867135, @efriedma wrote:

For the clang change, we should do something like D72579, not explicitly check for a specific target in target-independent code.

fix typo found by @dim

Updated comment

In D71600#1816160, @MaskRay wrote:

I am still confused why you need the special rule for __atomic_is_lock_free (GCC/clang) and __c11_atomic_is_lock_free (clang).

https://github.com/gcc-mirror/gcc/blob/master/gcc/builtins.c#L7300-L7314 GCC __atomic_is_lock_free expands to either 1 or a library call to __atomic_is_lock_free, never 0. How did FreeBSD get around libatomic when it was still using GCC? In clang, We can probably extend the semantics of __c11_atomic_is_lock_free and use clang::TargetInfo::getMaxAtomicPromoteWidth, but I'll make more research in this area.

Can you be elaborate how do __atomic_is_lock_free/__c11_atomic_is_lock_free pull in libatomic.{a,so} dependency on FreeBSD powerpc?

In D72306#1808613, @MaskRay wrote:

Obsoleted by D72352 ?

LGTM. Just a comment regarding FreeBSD 11.x.

In D72306#1807097, @MaskRay wrote:

Are you still using -target powerpc64-unknown-freebsd12.0-elfv2 or -target powerpc64-unknown-freebsd12.0-elfv1 added in https://reviews.llvm.org/rL361355 ?

As I'm not a commiter, may someone commit it, if it's the case, please?

update fixing the typo found during review.

In D67190#1670325, @jdoerfert wrote:

Is there a good way to test this? If so, we should, if not, this seems fine to me (except the typo @hbae found).

This solution works for me.
Validated it on FreeBSD13-CURRENT/powerpc64, with 32 bit compatibility libraries built using this patch over LLVM9.

Great job, @MaskRay !!

In D61950#1504169, @MaskRay wrote:

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.

• remove redundant comment

In D61610#1496129, @MaskRay wrote:

@adalava We care about the link time. I need to do some testing to ensure this doesn't cause regression. I've created https://reviews.llvm.org/D61720, fixed the tests and added ppc64-long-branch-init.s that I think captures the idea of the fix. Hope you don't mind my commandeering the change :)

In D61647#1496067, @MaskRay wrote:

See https://reviews.llvm.org/D61718 , not many tests need updating. The dump still diverges from GNU objdump in a few ways, e.g. the width of the size field, the number of spaces, the 'g' symbol etc

update testcase to use readelf instead of objdump, so it doesn't depend on D61647 anymore

In D61647#1494441, @MaskRay wrote:

This change makes sense and it will improve compatibility with GNU objdump. For your tests (I've commented there), I usually prefer llvm-readelf/llvm-readobj to llvm-objdump. The output of objdump also looks very weird to me. I remember @emaste said so in one FreeBSD issue.. I can check how many tests (you should at least check ninja -C Release check-llvm check-lld; clang has <5 llvm-objdump tests; ) will be impacted by this change. If there aren't many, I can fix them and commit that for you :)

In D59436#1494416, @MaskRay wrote:

Thanks. The test contains some constructs that are not used, e.g. .symver .addrsig. And I think it is usually better to use llvm-readelf/llvm-readobj. I always find the output of llvm-objdump/objdump weird.. If you don't mind, I can rewrite/minimalize the test and commit for you.

increase to PPC64_REL24 range

In D61647#1494070, @sfertile wrote:

This definitely seems a useful upgrade to me. Unfortunately it might affect a lot of lit tests. If we do intend to make a change we should try to match the output of the binutils objdump. I can see from the surrounding context we print out '.hidden', having a quick look it seems binutils will print out the visibility if its the only thing set, and the hex value otherwise (ie not both) here is an example:

Thank you @MaskRay, a test case was added. Please note this patch currently depends on D56586 and D61647 that need to go in first.

add test case

Thank you guys. I updated the patch,;

fix patch

• add space reservation, add comments and improve test

In rG70f25ebc6d1588bdf7fa07e7c42fa3c9a075101d#639599, @lebedev.ri wrote:

It was brought up before.
phab for some reason counts all the commits to all the github forks of the official github llvm monorepo like this

Sorry, I was in the wrong repository! It was meant to be in my private one.

Example code: