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

Support ARMv6 (original Raspberry Pi), ARMv7 bugfixes #901

Merged
merged 1 commit into from Jan 30, 2016
Merged

Support ARMv6 (original Raspberry Pi), ARMv7 bugfixes #901

merged 1 commit into from Jan 30, 2016

Conversation

hpux735
Copy link
Contributor

@hpux735 hpux735 commented Jan 7, 2016

this pull request adds support for ARMv6 (including the original Raspberry pi), and fixes several bugs encountered on ARMv7. I've been using this code for a few weeks, and is has been rather stable. My hope is that AMRv6 and ARMv7 on linux can be included in the Swift 2.2 release.

@jrose-apple
Copy link
Contributor

I'm not really the right person to review (just trying to get past the easy stuff), but what tests currently are and aren't passing?

@hpux735
Copy link
Contributor Author

hpux735 commented Jan 7, 2016

ARMv7 test results:


Testing Time: 754.18s


Failing Tests (28):
Swift :: 1_stdlib/ArrayCore.swift
Swift :: 1_stdlib/Interval.swift
Swift :: 1_stdlib/Optional.swift
Swift :: 1_stdlib/Range.swift
Swift :: 1_stdlib/Reflection.swift
Swift :: 1_stdlib/VarArgs.swift
Swift :: ClangModules/autolinking.swift
Swift :: DebugInfo/ASTSection_ObjC.swift
Swift :: DebugInfo/variables.swift
Swift :: Driver/Dependencies/one-way-external.swift
Swift :: Driver/subcommands.swift
Swift :: IRGen/c_layout.sil
Swift :: Interpreter/archetype_casts.swift
Swift :: Interpreter/formal_access.swift
Swift :: Interpreter/generic_class.swift
Swift :: Interpreter/generic_struct.swift
Swift :: Interpreter/protocol_extensions.swift
Swift :: Interpreter/protocol_lookup.swift
Swift :: Interpreter/slices.swift
Swift :: Interpreter/testability.swift
Swift :: Misc/expression_too_complex.swift
Swift :: Parse/BOM.swift
Swift :: Prototypes/CollectionsMoveIndices.swift
Swift :: Prototypes/FloatingPoint.swift
Swift :: Prototypes/MutableIndexableDict.swift
Swift :: Prototypes/TextFormatting.swift
Swift :: Serialization/autolinking.swift
Swift-Unit :: runtime/SwiftRuntimeTests/MetadataTest.getExistentialMetadata

Expected Passes : 1611
Expected Failures : 78
Unsupported Tests : 639
Unexpected Failures: 28

@hpux735
Copy link
Contributor Author

hpux735 commented Jan 8, 2016

ARMv6 results:


Testing Time: 7199.76s


Failing Tests (30):
Swift :: 1_stdlib/ArrayCore.swift
Swift :: 1_stdlib/FloatingPointIR.swift
Swift :: 1_stdlib/Interval.swift
Swift :: 1_stdlib/Optional.swift
Swift :: 1_stdlib/Range.swift
Swift :: 1_stdlib/Reflection.swift
Swift :: 1_stdlib/VarArgs.swift
Swift :: ClangModules/autolinking.swift
Swift :: DebugInfo/ASTSection_ObjC.swift
Swift :: DebugInfo/variables.swift
Swift :: Driver/Dependencies/one-way-external.swift
Swift :: Driver/subcommands.swift
Swift :: IRGen/c_layout.sil
Swift :: IRGen/multithread_module.swift
Swift :: Interpreter/archetype_casts.swift
Swift :: Interpreter/formal_access.swift
Swift :: Interpreter/generic_class.swift
Swift :: Interpreter/generic_struct.swift
Swift :: Interpreter/protocol_extensions.swift
Swift :: Interpreter/protocol_lookup.swift
Swift :: Interpreter/slices.swift
Swift :: Interpreter/testability.swift
Swift :: Misc/expression_too_complex.swift
Swift :: Parse/BOM.swift
Swift :: Prototypes/CollectionsMoveIndices.swift
Swift :: Prototypes/FloatingPoint.swift
Swift :: Prototypes/MutableIndexableDict.swift
Swift :: Prototypes/TextFormatting.swift
Swift :: Serialization/autolinking.swift
Swift-Unit :: runtime/SwiftRuntimeTests/MetadataTest.getExistentialMetadata

Expected Passes : 1609
Expected Failures : 78
Unsupported Tests : 639
Unexpected Failures: 30

@@ -159,7 +159,7 @@ function(_add_variant_link_flags
result)

if("${sdk}" STREQUAL "LINUX")
list(APPEND result "-lpthread" "-ldl")
list(APPEND result "-lpthread" "-ldl" "-Wl,-Bsymbolic")
Copy link
Collaborator

Choose a reason for hiding this comment

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

-Bsymbolic seems to be a big hammer (and it might be just a workaround for what is an LLVM or a linker bug). Could you scope it as narrowly as possible, just to armv6?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is. After 2.2 is forked, I'm going to work with @tienex on a different (probably better) solution based upon the gold linker. I'll restrict this to arm-linux.

@gribozavr
Copy link
Collaborator

@hpux735 The general approach is to run %target-swiftc_driver -### and add appropriate options that you would expect to trigger this behavior. The driver in -### will print the command line, but not execute it. I think in this case the changes you made will affect the linker options, so the tests should go into test/Driver/linker.swift -- or you might just need to add a few extra CHECK lines.

// LINUX-armv6-DAG: [[OBJECTFILE]]
// LINUX-armv6-DAG: -lswiftCore
// LINUX-armv6-DAG: -L [[STDLIB_PATH:[^ ]+/lib/swift]]
// LINUX-armv6-DAG: --target=junk-unknown-linux-gnueabihf
Copy link
Contributor Author

Choose a reason for hiding this comment

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

While attempting to add testing for whether --target is added to the clang++ invocation, this seems like the correct approach. However, this does not cause a failure as expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When lit is running a test on a file with more than one test (such as linker.swift), it looks like it bails when it finds the first failure, right? I deleted the tests relating to macos and the relative linker and verified that the tests for providing --target to clang functions as intended.

@hpux735
Copy link
Contributor Author

hpux735 commented Jan 23, 2016

I'm almost done with a patch that will do away with the swift.ld linker script, but in the spirit of small changes, can we get this moved forward so I can open up a new PR? In your opinion (@gribozavr )? what are the remaining issues that need to be addressed?

Thanks!!

@gribozavr
Copy link
Collaborator

Sure! Could you rebase the patch? (there are conflicts)

It also seems that @jckarter added -Wl,-Bsymbolic to the build anyway.

triple.getSubArch() == llvm::Triple::SubArchType::ARMSubArch_v6) {
triple.setArchName("armv6");
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit-pick: Perhaps combine some of these conditionals, and use else if? For example:

if (triple.getOS() == llvm::Triple::Linux && triple.getArch() == llvm::Triple::arm) {
  if (triple.getSubArch() == llvm::Triple::SubArchType::ARMSubArch_v7) {
    triple.setArchName("armv7");
  } else if (triple.getSubArch() == llvm::Triple::SubArchType::ARMSubArch_v6) {
    triple.setArchName("armv6");
  }
}

Personally I find this more readable, but that's just my two cents. 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Totally agree.

@hpux735
Copy link
Contributor Author

hpux735 commented Jan 27, 2016

I has conflicts with apple/master and resulting regressions. I'm working on resolving them at the moment.

@hpux735
Copy link
Contributor Author

hpux735 commented Jan 29, 2016

I believe that all the regression issues have been resolved. I think this is ready to go, finally.

@modocache
Copy link
Collaborator

@hpux735 Fantastic! Are all tests passing? If not, could you post a list of which aren't? I'd like to help if possible.

Also, I have a feeling someone on the core team will ask you to squash these commits--or at least rebase onto master, to get rid of intermediate commits like these:

screen shot 2016-01-29 at 9 12 56 am

But I'm not sure, so don't take my word for it. 😛

// LINUX-armv6-DAG: [[OBJECTFILE]]
// LINUX-armv6-DAG: -lswiftCore
// LINUX-armv6-DAG: -L [[STDLIB_PATH:[^ ]+/lib/swift]]
// LINUX-armv6-DAG: --target=armv6-unknown-linux-gnueabihf
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's the test you asked for, @gribozavr

@gribozavr
Copy link
Collaborator

@swift-ci Please test

@gribozavr
Copy link
Collaborator

@hpux735 Sorry, this failed on OS X:

TEST 'Swift :: Driver/linker.swift' FAILED
swift/test/Driver/linker.swift:135:17: error: expected string not found in input
// LINUX-armv6: swift
                ^
<stdin>:4:1: note: scanning from here

Do you have access to an OS X machine to investigate?

@hpux735
Copy link
Contributor Author

hpux735 commented Jan 29, 2016

Sorry about that! I just fixed an issue and ran it on my OSX machine. I think it will pass this time, it did for me.

@gribozavr
Copy link
Collaborator

@swift-ci Please test

@hpux735
Copy link
Contributor Author

hpux735 commented Jan 29, 2016

Is there a list of machines/platforms used for CI and the arguments used? I definitely don't want to waste people's time with these problems, if it can be avoided. I just tried it again on my mac and the tests completed without error.

@gribozavr
Copy link
Collaborator

@hpux735 It failed because of infrastructure reasons. Linux passed, we don't know about OS X. We'll get it fixed and I will re-run.

@gribozavr
Copy link
Collaborator

@swift-ci Please test

@gribozavr
Copy link
Collaborator

It failed with:

Failing Tests (1):
    Swift :: IDE/crashers_fixed/019-swift-vardecl-emitlettovarnoteifsimple.swift

which I fixed.

gribozavr added a commit that referenced this pull request Jan 30, 2016
Support ARMv6 (original Raspberry Pi), ARMv7 bugfixes
@gribozavr gribozavr merged commit b51bc1c into apple:master Jan 30, 2016
@gribozavr
Copy link
Collaborator

@hpux735 Thank you for working on the Linux arm port!

@hpux735
Copy link
Contributor Author

hpux735 commented Jan 30, 2016

Awesome!!! Thanks so much Dmitri!

@MacMeDan
Copy link

MacMeDan commented Feb 1, 2016

@hpux735 you are my hero.

@hpux735
Copy link
Contributor Author

hpux735 commented Feb 1, 2016

LOL, Glad I could help, @peterdan

finagolfin added a commit to finagolfin/swift that referenced this pull request Dec 9, 2020
The target was added for Unix toolchains in apple#901, but a later pull apple#1891 added
it again. Since clang only uses the last target flag that's passed in, all
customization done for the first one was unused these last 4+ years, so remove
it and change tests that look for custom strings passed by the first one.
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

5 participants