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

TSCUtility: support additional spellings for ARM #337

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

compnerd
Copy link
Member

ARM uses armv[4-9]-?[arm] as the arch field to encode the ISA
version and the core type for ARMv7. Use the custom parsing logic
similar to the other fields to accommodate the spelling.

@compnerd
Copy link
Member Author

@swift-ci please test

]
if let match = candidates.first(where: { string.hasPrefix($0.key) })?.value {
if case let .armv7(core: _) = match {
if string.hasPrefix("armv7-a") || string.hasPrefix("armv7a") {
Copy link
Contributor

Choose a reason for hiding this comment

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

The init function won't accept armv7-a because it splits the triple into components delimited on -, (line 114) then you'll either read the a as the vendor, or emit an error because the triple has the wrong number of components.

Copy link
Contributor

@etcwilde etcwilde left a comment

Choose a reason for hiding this comment

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

I think this is a good start. I kind of wonder if it shouldn't be generalized more than just for the core? There are a bunch of sub-architectures under armv7 alone. It might make sense really to make the arm case take an optional ARM sub arch, which would then include v7, v7a, v7s, v7k, etc... Thoughts?

ARM uses `armv[4-9]-?[arm]` as the arch field to encode the ISA
version and the core type for ARMv7.  Use the custom parsing logic
similar to the other fields to accommodate the spelling.
@compnerd
Copy link
Member Author

@swift-ci please test

@@ -283,6 +329,78 @@ extension Triple {
}
}

extension Triple.Arch: CustomStringConvertible {
public var description: String {
switch self {
Copy link
Contributor

Choose a reason for hiding this comment

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

More of a nit, but could we utilize the candidate array from parseArch() here for the non-armv7 cases? I think that would simplify the maintenance a bit.

@tomerd tomerd added the wip Work in progress label Jul 18, 2022
"arm64e": .arm64e,
"wasm32": .wasm32,
]
if let match = candidates.first(where: { string.hasPrefix($0.key) })?.value {

Choose a reason for hiding this comment

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

Swift's Dictionary isn’t ordered, so we will run into random results here by using first(where:).

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

Successfully merging this pull request may close these issues.

5 participants