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

PS4 support #3221

Merged
merged 1 commit into from
Jul 1, 2016
Merged

PS4 support #3221

merged 1 commit into from
Jul 1, 2016

Conversation

compnerd
Copy link
Member

What's in this pull request?

Resolved bug number: (SR-)


Before merging this pull request to apple/swift repository:

  • Test pull request on Swift continuous integration.

Triggering Swift CI

The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:

Smoke Testing

Platform Comment
All supported platforms @swift-ci Please smoke test
All supported platforms @swift-ci Please smoke test and merge
OS X platform @swift-ci Please smoke test OS X platform
Linux platform @swift-ci Please smoke test Linux platform

Validation Testing

Platform Comment
All supported platforms @swift-ci Please test
All supported platforms @swift-ci Please test and merge
OS X platform @swift-ci Please test OS X platform
OS X platform @swift-ci Please benchmark
Linux platform @swift-ci Please test Linux platform

Lint Testing

Language Comment
Python @swift-ci Please Python lint

Note: Only members of the Apple organization can trigger swift-ci.

@jrose-apple
Copy link
Contributor

cc @gribozavr, who's shepherded most new platforms.

How do PS4 version numbers work? It does seem to make sense that PS3 and PS4 are completely separate OSs, but…

@gribozavr
Copy link
Contributor

gribozavr commented Jun 26, 2016

Please add two tests like ./test/Parse/ConditionalCompilation/armAndroidTarget.swift and ./validation-test/StdlibUnittest/Android.swift.

I think every PS* platform should be treated as a separate target; for the existing platforms we know that every PS* target runs a unique OS, and it is ABI-incompatible and probably ISA-incompatible to every other PS* target. Even if the new one can run the programs for the previous one, it is typically an emulation mode that for all compilation concerns looks like the previous environment.

#if os(FreeBSD)
// This block should not parse.
// os(FreeBSD) does not imply os(PS4)
let i : Int = "Hello"
Copy link
Contributor

Choose a reason for hiding this comment

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

There should be no space before ":".

@compnerd
Copy link
Member Author

Actually, lets wait for #3218 so that I can rebase on that and fix the linker directive handling. But, Ive added the test, any other review comments would be appreciated so that I can fix it up in one go.

@gribozavr
Copy link
Contributor

@swift-ci Please test and merge

@gribozavr
Copy link
Contributor

@compnerd Please take a look at the CI issues:

/home/buildnode/jenkins/workspace/swift-PR-Linux/swift/validation-test/StdlibUnittest/PS4.swift:7:24: error: type 'OSVersion' has no member 'PS4'

@compnerd compnerd force-pushed the ps4 branch 2 times, most recently from 76b760f to b1c245a Compare June 28, 2016 14:43
@gribozavr
Copy link
Contributor

@swift-ci Please test and merge

@gribozavr
Copy link
Contributor

@compnerd

/Users/buildnode/jenkins/workspace/swift-PR-osx/swift/stdlib/private/StdlibUnittest/StdlibUnittest.swift.gyb:1616:35: error: use of unresolved identifier 'reasaon'
      return "ps4Any(*, reason: \(reasaon))"
                                  ^~~~~~~
/Users/buildnode/jenkins/workspace/swift-PR-osx/swift/stdlib/private/StdlibUnittest/StdlibUnittest.swift.gyb:1615:30: note: did you mean 'reason'?
    case .ps4Any(reason: let reason):

@compnerd
Copy link
Member Author

The test needs to be fixed, and I believe I need to hunt down one more issue in the linker option emission for PS4 work to be ready to merge.

@gribozavr
Copy link
Contributor

@compnerd Sounds good. Just @-mention me when you are ready.

Add support for the PS4 OS.  Update the standard library and add a target unit
test.
@compnerd
Copy link
Member Author

compnerd commented Jul 1, 2016

@swift-ci Please test

@compnerd
Copy link
Member Author

compnerd commented Jul 1, 2016

@gribozavr I think that all the swift side of things are complete.

@compnerd
Copy link
Member Author

compnerd commented Jul 1, 2016

@gribozavr if nothing else remains to be addressed, I think this can be merged.

buffer += '"';
buffer += library;
if (quote)
buffer += '"';
Copy link
Contributor

Choose a reason for hiding this comment

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

This matches what Clang does in its PS4TargetCodeGenInfo, so LGTM.

@gribozavr
Copy link
Contributor

@swift-ci Please test and merge

@swift-ci swift-ci merged commit f235a62 into swiftlang:master Jul 1, 2016
@CodaFi
Copy link
Contributor

CodaFi commented Jul 1, 2016

@compnerd Do you know how the PS4 represents process arguments?

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.

5 participants