Skip to content

Adding support for ARMv7 on linux #159

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Dec 18, 2015
Merged

Adding support for ARMv7 on linux #159

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4737e7c
Adding support for ARMv7 on linux
hpux735 Dec 18, 2015
779d806
Quickly (and hackily) patched up some word size issues
hpux735 Dec 18, 2015
06cf35a
Reverting changes to API
hpux735 Dec 18, 2015
f57d960
Beginning to address PR issues.
hpux735 Dec 18, 2015
e63374d
Continuing to address PR issues
hpux735 Dec 18, 2015
24ef52e
Added a reference to JIRA SR-314
hpux735 Dec 18, 2015
b46d244
Backed-out experimental cpu option setting
hpux735 Dec 18, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 1 addition & 9 deletions CoreFoundation/Base.subproj/CFBase.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -875,15 +875,7 @@ void _CFRuntimeSetCFMPresent(void *addr) {


extern void __HALT() {
#if defined(__ppc__)
__asm__("trap");
#elif defined(__i386__) || defined(__x86_64__)
#if defined(_MSC_VER)
__asm int 3;
#else
__asm__("int3");
#endif
#endif
__builtin_trap();
}


8 changes: 8 additions & 0 deletions CoreFoundation/Base.subproj/CFInternal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,14 @@ CF_PRIVATE CFIndex __CFActiveProcessorCount();
#else
#error Compiler not supported
#endif
#elif defined(__ppc__) || (__arm__)
#if defined(__GNUC__)
#define HALT do {asm __volatile__("trap"); kill(getpid(), 9); __builtin_unreachable(); } while (0)
#elif defined(_MSC_VER)
#define HALT do { DebugBreak(); abort(); __builtin_unreachable(); } while (0)
#else
#error Compiler not supported
#endif
#endif

#if defined(DEBUG)
Expand Down
16 changes: 11 additions & 5 deletions Foundation/NSXMLParser.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@
// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
//

// It is necessary to explicitly cast strlen to UInt to match the type
// of prefixLen because currently, strlen (and other functions that
// rely on swift_ssize_t) use the machine word size (int on 32 bit and
// long in on 64 bit). I've filed a bug at bugs.swift.org:
// https://bugs.swift.org/browse/SR-314

#if os(OSX) || os(iOS)
import Darwin
#elseif os(Linux)
Expand Down Expand Up @@ -233,9 +239,9 @@ internal func _NSXMLParserStartElementNs(ctx: _CFXMLInterface, localname: Unsafe
let parser = ctx.parser
let reportQNameURI = parser.shouldProcessNamespaces
let reportNamespaces = parser.shouldReportNamespacePrefixes
let prefixLen = prefix == nil ? strlen(UnsafePointer<Int8>(prefix)) : 0
let prefixLen = prefix == nil ? UInt(strlen(UnsafePointer<Int8>(prefix))) : 0
Copy link
Contributor

Choose a reason for hiding this comment

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

isn't the declaration of strlen
size_t strlen(const char *s)
which size_t should be unsigned in any reasonable environment typedefed by __SIZE_TYPE__ hopefully or unsigned long if that isn't the case

Which os defines it as int strlen(const char *s)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In swift/stdlib/public/SwiftShims/LibcShims.h swift_ssize_t is defined

#if defined(__linux__) && defined (__arm__)
typedef      int __swift_ssize_t;
#else
 typedef long int __swift_ssize_t;
#endif

and later in swift/stdlib/public/stubs/LibcShims.cpp it is checked with a static assert to ensure that swift_ssize_t matches whatever the C ssize_t is. Because of this, the size of ssize_t cannot be assumed to be a long int.

static_assert(std::is_same<ssize_t, swift::__swift_ssize_t>::value,
              "__swift_ssize_t is wrong");

I definitely don't think that this commit is the right way to do it. This is another case of trying to get it to finish building to check linking. I can revert this to what's existing for the purposes of this pull request.

Copy link
Contributor

Choose a reason for hiding this comment

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

nah; if that is the current definition then lets roll with it and just submit a bug report on bugs.swift.org with perhaps a comment in the top of this file to explain why that is the case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done: https://bugs.swift.org/browse/SR-314

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apparently this is intended behavior. Jordan Rose closed the bug report.

Copy link
Contributor

Choose a reason for hiding this comment

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

strlen() is returning size_t, not ssize_t (note the double s). I think this issue is worth further investigation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although it's true that size_t (and swift_size_t) is distinct from swift_ssize_t it still changes size based upon the underlying architecture. It is ultimately typedef'd from __SIZE_TYPE__ which, although I couldn't find its actual definition, is 8 bytes on x86_64 (linux or mac) and 4 bytes on 32-bit arm.

I still don't think this is the best solution in this particular instance. The error we're addressing here is:

error: cannot convert value of type 'UInt32' to expected argument type 'UInt'

So, it is my expectation that we're promoting a 32-bit unsigned int to a 64-bit unsigned int, or simply removing the bit-size specialization, with it remaining 32-bits. Either way, it's safe in the 32-bit case. In 64-bit code, this should be a no-op, because it's a cast from UInt to UInt.

Copy link
Contributor

Choose a reason for hiding this comment

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

size_t is supposed to be imported as Int in Swift, the importer has a special case for it. Would you mind checking if there is an issue with that on Linux/armv7?

import Glibc
print(size_t.self) // should print '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.

Yes, It is.

wdillon@tegra-ubuntu:~$ cat size_t.swift 
import Glibc
print(size_t.self) // should print 'Int'

wdillon@tegra-ubuntu:~$ swiftc size_t.swift 
wdillon@tegra-ubuntu:~$ ./size_t 
Int
wdillon@tegra-ubuntu:~$ uname -s -m
Linux armv7l

At some pint strlen's return value becomes Uint32.

let localnameString = (prefixLen == 0 || reportQNameURI) ? UTF8STRING(localname) : nil
let qualifiedNameString = prefixLen != 0 ? _colonSeparatedStringFromPrefixAndSuffix(prefix, prefixLen, localname, strlen(UnsafePointer<Int8>(localname))) : localnameString
let qualifiedNameString = prefixLen != 0 ? _colonSeparatedStringFromPrefixAndSuffix(prefix, UInt(prefixLen), localname, UInt(strlen(UnsafePointer<Int8>(localname)))) : localnameString
let namespaceURIString = reportQNameURI ? UTF8STRING(URI) : nil

var nsDict = [String:String]()
Expand All @@ -248,7 +254,7 @@ internal func _NSXMLParserStartElementNs(ctx: _CFXMLInterface, localname: Unsafe
if reportNamespaces {
namespaceNameString = UTF8STRING(namespaces[idx])
}
asAttrNamespaceNameString = _colonSeparatedStringFromPrefixAndSuffix("xmlns", 5, namespaces[idx], strlen(UnsafePointer<Int8>(namespaces[idx])))
asAttrNamespaceNameString = _colonSeparatedStringFromPrefixAndSuffix("xmlns", 5, namespaces[idx], UInt(strlen(UnsafePointer<Int8>(namespaces[idx]))))
} else {
namespaceNameString = ""
asAttrNamespaceNameString = "xmlns"
Expand Down Expand Up @@ -280,7 +286,7 @@ internal func _NSXMLParserStartElementNs(ctx: _CFXMLInterface, localname: Unsafe
let attrPrefix = attributes[idx + 1]
let attrPrefixLen = attrPrefix == nil ? strlen(UnsafePointer<Int8>(attrPrefix)) : 0
if attrPrefixLen != 0 {
attributeQName = _colonSeparatedStringFromPrefixAndSuffix(attrPrefix, attrPrefixLen, attrLocalName, strlen((UnsafePointer<Int8>(attrLocalName))))
attributeQName = _colonSeparatedStringFromPrefixAndSuffix(attrPrefix, UInt(attrPrefixLen), attrLocalName, UInt(strlen((UnsafePointer<Int8>(attrLocalName)))))
} else {
attributeQName = UTF8STRING(attrLocalName)!
}
Expand Down Expand Up @@ -318,7 +324,7 @@ internal func _NSXMLParserEndElementNs(ctx: _CFXMLInterface , localname: UnsafeP
let prefixLen = prefix == nil ? strlen(UnsafePointer<Int8>(prefix)) : 0
let localnameString = (prefixLen == 0 || reportQNameURI) ? UTF8STRING(localname) : nil
let nilStr: String? = nil
let qualifiedNameString = (prefixLen != 0) ? _colonSeparatedStringFromPrefixAndSuffix(prefix, prefixLen, localname, strlen(UnsafePointer<Int8>(localname))) : nilStr
let qualifiedNameString = (prefixLen != 0) ? _colonSeparatedStringFromPrefixAndSuffix(prefix, UInt(prefixLen), localname, UInt(strlen(UnsafePointer<Int8>(localname)))) : nilStr
let namespaceURIString = reportQNameURI ? UTF8STRING(URI) : nilStr


Expand Down
3 changes: 2 additions & 1 deletion configure
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,4 +179,5 @@ def main():


if __name__ == "__main__":
main()
main()

136 changes: 111 additions & 25 deletions lib/target.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,99 @@ class ArchType:
spir64 = 37
kalimba = 38
shave = 39

# Do not assume that these are 1:1 mapping. This should follow
# canonical naming conventions for arm, etc. architectures.
# See apple/swift PR #608
@staticmethod
def to_string(value):
if value == ArchType.arm:
return "armv7"
Copy link
Contributor

Choose a reason for hiding this comment

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

isn't armv7a the more common version?

if value == ArchType.armeb:
return "armeb"
if value == ArchType.aarch64:
return "aarch64"
if value == ArchType.aarch64_be:
return "aarch64_be"
if value == ArchType.bpfel:
return "bpfel"
if value == ArchType.bpfeb:
return "bpfeb"
if value == ArchType.hexagon:
return "hexagon"
if value == ArchType.mips:
return "mips"
if value == ArchType.mipsel:
return "mipsel"
if value == ArchType.mips64:
return "mips64"
if value == ArchType.mips64el:
return "mips64el"
if value == ArchType.msp430:
return "msp430"
if value == ArchType.ppc:
return "ppc"
if value == ArchType.ppc64:
return "ppc64"
if value == ArchType.ppc64le:
return "ppc64le"
if value == ArchType.r600:
return "r600"
if value == ArchType.amdgcn:
return "amdgcn"
if value == ArchType.sparc:
return "sparc"
if value == ArchType.sparcv9:
return "sparcv9"
if value == ArchType.sparcel:
return "sparcel"
if value == ArchType.systemz:
return "systemz"
if value == ArchType.tce:
return "tce"
if value == ArchType.thumb:
return "armv7"
if value == ArchType.thumbeb:
return "thumbeb"
if value == ArchType.x86:
return "i386"
if value == ArchType.x86_64:
return "x86_64"
if value == ArchType.xcore:
return "xcore"
if value == ArchType.nvptx:
return "nvptx"
if value == ArchType.nvptx64:
return "nvptx64"
if value == ArchType.le32:
return "le32"
if value == ArchType.le64:
return "le64"
if value == ArchType.amdil:
return "amdil"
if value == ArchType.amdil64:
return "amdil64"
if value == ArchType.hsail:
return "hsail"
if value == ArchType.hsail64:
return "hsail64"
if value == ArchType.spir:
return "spir"
if value == ArchType.spir64:
return "spir64"
if value == ArchType.kalimba:
return "kalimba"
if value == ArchType.shave:
return "shave"
return "unknown"
# Not 1:1, See to_string
@staticmethod
def from_string(string):
if string == "arm":
return ArchType.arm
# Match big endian arm first
if string == "armeb":
return ArchType.armeb
# Catch-all for little endian arm
if "arm" in string:
return ArchType.arm
if string == "aarch64":
return ArchType.aarch64
if string == "aarch64_be":
Expand Down Expand Up @@ -223,8 +309,8 @@ class Vendor:

class Target:
triple = None
sdk = OSType.MacOSX
arch = ArchType.x86_64
sdk = None
arch = None
executable_suffix = ""
dynamic_library_prefix = "lib"
dynamic_library_suffix = ".dylib"
Copy link
Contributor

Choose a reason for hiding this comment

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

these are now incorrect if the sdk and arch are defaulted to unknown

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 think I added the appropriate logic to compensate below. The only issue I have with setting default values is that it increased the time it took to identify the logic bug that "ArchType.from_string(comps[0])" wasn't actually assigning anything, it was essentially a no-op. The default value made it look like everything was fine, just wrong.

Copy link
Contributor

Choose a reason for hiding this comment

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

fair point; I agree having an opinion about a "default" target is probably incorrect in a general sense. Just was thinking that perhaps it might be good to have these as None or some other thing to avoid any potential other trip-ups

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. I'll do that.

Expand All @@ -242,20 +328,30 @@ def __init__(self, triple):
self.sdk = OSType.Win32
self.dynamic_library_suffix = ".dll"
self.executable_suffix = ".exe"
elif "darwin" in triple:
self.sdk = OSType.MacOSX
else:
print("Unknown platform")

self.triple = triple

comps = triple.split('-')
ArchType.from_string(comps[0])
self.arch = ArchType.from_string(comps[0])

@staticmethod
def default():
triple = platform.machine() + "-"
arch = ArchType.from_string(platform.machine())
triple = ArchType.to_string(arch)
if platform.system() == "Linux":
triple += "linux-gnu"
if arch == ArchType.arm:
triple += "-linux-gnueabihf"
Copy link
Contributor

Choose a reason for hiding this comment

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

what about "-linux-armeabi" or "-linux-android" etc? I know these are other handled cases in llvm's triple calculations

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, I don't know the best way to handle that. Do you know a good way to detect what that part of the triple should be?

Copy link
Contributor

Choose a reason for hiding this comment

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

See http://clang.llvm.org/docs/CrossCompilation.html

arch = x86, arm, thumb, mips, etc.
sub = for ex. on ARM: v5, v6m, v7a, v7m, etc.
vendor = pc, apple, nvidia, ibm, etc.
sys = none, linux, win32, darwin, cuda, etc.
abi = eabi, gnu, android, macho, elf, etc.

So we probably need another field since triples are not three parts and instead five parts with three reasonably optional fields.
arch[sub][-vendor]-sys[-abi]

However this method is the script's host not the target. So I am not certain which hosts we want to really support here.

else:
triple += "-linux-gnu"
elif platform.system() == "Darwin":
triple += "apple-darwin"
triple += "-apple-darwin"
elif platform.system() == "FreeBSD":
# Make this working on 10 as well.
triple += "freebsd11.0"
# Make this work on 10 as well.
triple += "-freebsd11.0"
else:
# TODO: This should be a bit more exhaustive
print("unknown host os")
Expand All @@ -264,21 +360,17 @@ def default():

@property
def swift_triple(self):
triple = ""
if self.arch == ArchType.x86_64:
triple = "x86_64"
else:
print("unknown arch for swift")
return None
triple = ArchType.to_string(self.arch)
if self.sdk == OSType.MacOSX:
return None
elif self.sdk == OSType.Linux:
triple += "-pc-linux"
triple += "-unknown-linux"
elif self.sdk == OSType.FreeBSD:
triple += "-unknown-freebsd"
else:
print("unknown sdk for swift")
return None

return triple

@property
Expand All @@ -296,13 +388,7 @@ def swift_sdk_name(self):

@property
def swift_arch(self):
arch = ""
if self.arch == ArchType.x86_64:
arch = "x86_64"
else:
print("unknown arch for swift")
return None
return arch
return ArchType.to_string(self.arch)

class TargetConditional:
_sdk = None
Expand Down