Skip to content

Conversation

@tinysun212
Copy link
Contributor

Rebased PR from previous.
I made several changes set for cygwin64 port.

The standard library can be used and It is possible to compile and run or interpret a simple hello swift source, but REPL, swift-build and swift-lldb does not work.
Currently, build process is not good, and many bugs may exist in.

@tinysun212
Copy link
Contributor Author

I'll made more patches for stdlib/public/stubs/Stubs.cpp, stdlib/public/stubs/UnicodeNormalization.cpp,
include/swift/Basic/Dwarf.h.

@gribozavr gribozavr mentioned this pull request Jan 27, 2016
Copy link
Contributor

Choose a reason for hiding this comment

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

uintptr_t might be a better fit?

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 seems other implementations (Linux, Cygwin internal.. ) didn't use a pointer type, so I chosen that.

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't want a pointer, we want a pointer-sized int.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I misunderstand about uintptr_t. I'll change to it. Thank you.

@gribozavr
Copy link
Contributor

@tinysun212 Thank you for your updates! I made more comments, please take a look.

@gribozavr
Copy link
Contributor

This is looking really good now, by the way!

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you change TwoWordPair::Return in the runtime to be an __int128 on Win64 instead of exposing the C calling convention difference in Swift code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll try it.

@tinysun212
Copy link
Contributor Author

I'm waiting reviews for correction. If there is nothing to correct, what should I do with this PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

The variable should be declared as size_t instead of a cast on the line below.

@tinysun212
Copy link
Contributor Author

This PR conflicted again. I don't know how to resolve this. Should I rebase to top of master and recreate another PR ?

@gribozavr
Copy link
Contributor

@tinysun212 You can rebase, squash, and git push -f to the same branch just one commit. Github will automatically update the PR. Alternatively, you can submit a new PR, whichever is simpler.

@tinysolver
Copy link

I rebased and squashed all previous commits to one. And fixed some compile errors. Thank you.

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove commented code?

@gribozavr
Copy link
Contributor

@swift-ci Please test

@gribozavr
Copy link
Contributor

@tinysun212 Would you mind fixing the CMake issues that the buildbot found? You'd need to use set(LLVM_OPTIONAL_SOURCES ...) for the appropriate files.

@gribozavr
Copy link
Contributor

@swift-ci Please test

@tinysun212
Copy link
Contributor Author

Currently, I have Linux and Windows platforms for development and not OS X platform.
Could somebody tell me the point that cause the test failed or any hint to solve this ?

@gribozavr
Copy link
Contributor

@tinysun212 Sorry, the OS X tests failing is not your fault. The master branch is broken now, we are working to get it fixed. This should land imminently and we'll re-run tests then.

@gribozavr
Copy link
Contributor

@swift-ci Please test

@gribozavr
Copy link
Contributor

@tinysun212 I'm sorry, could you fix the conflicts? The patch LGTM, I will merge immediately!

@gribozavr
Copy link
Contributor

@tinysun212 Thank you!

@gribozavr
Copy link
Contributor

@swift-ci Please smoke test

@gribozavr
Copy link
Contributor

@tinysun212 CI found an issue on Linux:

CMake Error at /home/buildnode/jenkins/workspace/swift-PR-Linux@2/Ninja-ReleaseAssert/llvm-linux-x86_64/share/llvm/cmake/LLVMProcessSources.cmake:83 (message):
  Found unknown source file
  /home/buildnode/jenkins/workspace/swift-PR-Linux@2/swift/stdlib/public/runtime/CygwinPort.cpp

To fix, please merge the two definitions of LLVM_OPTIONAL_SOURCES into one (file stdlib/public/runtime/CMakeLists.txt).

@gribozavr
Copy link
Contributor

@swift-ci Please smoke test

@gribozavr
Copy link
Contributor

@tinysun212 Sorry, CI is not happy again. You need to merge the definition into the top one, because there is an add_swift_library in between.

@tinysun212
Copy link
Contributor Author

@gribozavr I'm sorry repetitive fail in same position. In this time, I tested the cmake-running (not build) in Linux.

@gribozavr
Copy link
Contributor

@swift-ci Please test

@tinysun212
Copy link
Contributor Author

OS X tests failed again. I think it is not my fault. Let me know if my action is needed.

@gribozavr
Copy link
Contributor

@tinysun212 Yes, that's existing breakage. Merging! Thank you for working on this!

gribozavr added a commit that referenced this pull request Feb 23, 2016
@gribozavr gribozavr merged commit 7235595 into swiftlang:master Feb 23, 2016
@modocache modocache mentioned this pull request Feb 25, 2016
@tinysun212 tinysun212 deleted the porting-to-cygwin branch October 21, 2018 00:14
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