Page MenuHomePhabricator

Change llvm call once check for building Swift for PowerPC(ppc64le)
ClosedPublic

Authored by sarveshtamba on Jan 7 2019, 3:22 AM.

Details

Reviewers
nemanjai
compnerd
Summary

These changes are required to resolve the call_once related errors seen while building the Swift toolchain on PowerPC64LE. The existing macro checks are inadequate while building Swift on PowerPC64LE.

Diff Detail

Repository
rL LLVM

Event Timeline

sarveshtamba created this revision.Jan 7 2019, 3:22 AM

@nemanjai, please review and let me know if this looks good.

@nemanjai, any updates on this one?

Did we not address the PPC related checks earlier? IIRC, the problem was that gcc does not define __PPC__ but does define __ppc__. The problem with std::call_once was not related to endian, so why not extend the condition on L32 to include __PPC__?

Extending the condition to include PPC

Hi @compnerd

In https://reviews.llvm.org/D55503, we had extended the condition to include PPC.
However, the check required to build LLVM on PPC64LE was missed in SVN r348970.

Hence as requested by @nemanjai in review request D55503, I have raised this new review request D56382. I have also extended the condition to include PPC by submitting an additional diff to this new review request D56382.

Hello, @nemanjai , @compnerd

Any updates on this?

compnerd accepted this revision.Jan 24 2019, 9:26 AM

Ah, okay. LGTM.

Do you need me to commit this on your behalf (as last time)?

This revision is now accepted and ready to land.Jan 24 2019, 9:26 AM

Sure @compnerd, please go ahead and commit this on my behalf like last time. Thanks in advance!

@sarveshtamba - just to make sure, you are aware that the license for LLVM has changed and that this work is acceptable to be published under the new license?

@compnerd - I will check this and let you know. In the meanwhile can you please point towards the new license documentation/details?

Sure, the new license is at https://llvm.org/LICENSE.txt (Apache)

@compnerd - I got a confirmation that we can go ahead and merge this. Please go ahead and commit this on my behalf.

Herald added a project: Restricted Project. · View Herald TranscriptFeb 11 2019, 9:03 PM

@compnerd - Please let me know once this is committed. Thanks.

compnerd closed this revision.Feb 14 2019, 10:36 AM

SVN r354045

@compnerd Has this been pushed upstream? I pulled latest code for Apple Swift master, and did not see this change in swift-llvm repository. Please confirm.

Can anyone confirm if this has been pushed upstream in the master branch? I pulled latest code for Apple Swift master, and did not see this change in swift-llvm repository.

jsji added a comment.Thu, Mar 21, 6:54 AM

@sarveshtamba As the author mentioned, yes, this has been upstreamed.

Note that this patch was actually splitted into two commits,
the first one in https://reviews.llvm.org/rL348970 for the __PPC__ macro,
the second one in https://reviews.llvm.org/rL354045.

However, looks like swift-llvm did not sync the repo timely( or correctly),
So it only contains rL348970 for now.

You'll either have to wait, or contact maintainer of swift-llvm to report the sync issue.