Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add powerpc64le Linux support #979

Merged
merged 1 commit into from Jan 16, 2016

Conversation

antonblanchard
Copy link
Contributor

This patch adds powerpc64le Linux support. While the patch also adds
the matching powerpc64 bits, there are endian issues that need to be
sorted out.

The PowerPC LLVM changes for the swift ABI (eg returning three element
non-homogeneous aggregates) are still in the works, but a simple LLVM
fix to allow those aggregates results in swift passing all but 8
test cases.

Note: It would be nice if swift had conditional compilation based on pointer size, a
number of the test cases only care if the target is 32 or 64 bit.

@gribozavr
Copy link
Collaborator

Is powerpc64 big-endian?

@antonblanchard
Copy link
Contributor Author

Yes, powerpc64 is big endian. We hope to get big endian working, but for now are concentrating on the little endian port (powerpc64le).

@@ -919,7 +919,7 @@ static const uintptr_t ObjCReservedBitsMask =
static const unsigned ObjCReservedLowBits =
SWIFT_ABI_X86_64_OBJC_NUM_RESERVED_LOW_BITS;

#elif defined(__arm64__)
#elif defined(__arm64__) || defined(__powerpc64__)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is __powerpc64__ defined on BE, LE or both?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is defined on both BE and LE targets:

$ uname -m
ppc64le
$ cpp -dM < /dev/null | grep __powerpc64__
#define __powerpc64__ 1
$ uname -m
ppc64
$ cpp -dM < /dev/null | grep __powerpc64__
#define __powerpc64__ 1

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment above this block says that lib/IRGen/SwiftTargetInfo.cpp needs adjustments (you need to add a configurePowerPC function there).

@gribozavr
Copy link
Collaborator

How far does this patch allows the port to go? Does the standard library build? Does hello world work? Do the tests run?

@antonblanchard
Copy link
Contributor Author

The powerpc64le port builds and runs the test suite with a few failures:

********************
Testing Time: 275.66s
********************
Failing Tests (5):
    Swift :: 1_stdlib/VarArgs.swift
    Swift :: Driver/filelists.swift
    Swift :: IRGen/c_layout.sil
    Swift :: IRGen/type_layout_reference_storage.swift
    Swift :: Parse/BOM.swift

  Expected Passes    : 1698
  Expected Failures  : 67
  Unsupported Tests  : 651
  Unexpected Failures: 5
*** Failed while running tests for swift (check-swift-linux-powerpc64le)

And hello world works:

$ cat hello.swift 
print("Hello, world!")
$ swiftc hello.swift 
$ ./hello 
Hello, world!

@gribozavr
Copy link
Collaborator

@antonblanchard I'm really impressed!

@gribozavr
Copy link
Collaborator

Did you try the validation test suite build-script -T?

@antonblanchard
Copy link
Contributor Author

Thanks, that's given me a few more failing tests to sort out:

********************
Testing Time: 1467.18s
********************
Failing Tests (8):
    Swift :: 1_stdlib/VarArgs.swift
    Swift :: Driver/filelists.swift
    Swift :: IRGen/c_layout.sil
    Swift :: IRGen/type_layout_reference_storage.swift
    Swift :: Parse/BOM.swift
    Swift :: stdlib/AtomicInt.swift
    Swift :: stdlib/CollectionType.swift.gyb
    Swift :: stdlib/FloatingPointConversionTraps.swift.gyb

  Expected Passes    : 6961
  Expected Failures  : 79
  Unsupported Tests  : 771
  Unexpected Failures: 8

@gribozavr
Copy link
Collaborator

Again, very impressed it is just 8 tests in total!

@antonblanchard
Copy link
Contributor Author

Yes, I'm surprised by how clean the port is. Having the compiler written in c++ (vs the target language), and pulling in clang to get the ABI parameter passing rules helps. Other LLVM based languages have required a lot more work.

Big endian still requires work, but we can attack that once the little endian port is solid.

@antonblanchard
Copy link
Contributor Author

I'm not sure of the preferred workflow, but I'm happy to rebase and squash these commits.

self = ${Self}(Builtin.int_bswap_${BuiltinName}(value._value) )
#else
_UnsupportedArchitectureError()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep the unsupported error so that we get a loud error for new architectures. Just add an explicit #elseif arch(powerpc64) for BE (here and below).

@gribozavr
Copy link
Collaborator

Yes, we prefer rebasing/squashing.

break;
case llvm::Triple::ArchType::ppc64le:
addTargetConfigOption("arch", "powerpc64le");
break;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part needs a test like test/BuildConfigurations/x64LinuxTarget.swift for each supported triple.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, will add these tests

@gribozavr
Copy link
Collaborator

@antonblanchard Thank you! Looks good, but I have two more comments.

@lattner
Copy link
Collaborator

lattner commented Jan 15, 2016

Very cool, thank you for working on adding this!

This patch adds powerpc64le Linux support. While the patch also adds
the matching powerpc64 bits, there are endian issues that need to be
sorted out.

The PowerPC LLVM changes for the swift ABI (eg returning three element
non-homogeneous aggregates) are still in the works, but a simple LLVM
fix to allow those aggregates results in swift passing all but 8
test cases.
@gribozavr
Copy link
Collaborator

@antonblanchard The ObjC values should be dynamically dead as far as I understand. If you find they are also statically unused -- feel free to remove them!

@gribozavr
Copy link
Collaborator

I ran tests on Ubuntu 14.04 x86_64 and OS X, and see no regressions. Merging. Thank you!

gribozavr added a commit that referenced this pull request Jan 16, 2016
@gribozavr gribozavr merged commit 5a1f8b2 into apple:master Jan 16, 2016
@antonblanchard
Copy link
Contributor Author

Thanks for your help @gribozavr!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants