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.

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

Jump to bottom

Add support for architectures in the wrappers #4

Merged
merged 4 commits into from
Jun 18, 2020
Merged

Add support for architectures in the wrappers #4

merged 4 commits into from
Jun 18, 2020

Conversation

giordano
Copy link
Member

@giordano giordano commented Jun 14, 2020 •
edited
Loading

This is still very much work in progress

Still to do:

  • Need to define an extended_platform_key_abi()?
  • Extend gcc_version to take march into account?
  • Inform auditor about the march
    • Fix is_for_platform to use base_platform defined in this PR. It should use arch() function instead of accessing .arch field
  • Teach minimum_instruction_set more instruction sets
    • It would be great to issue a warning if we're building for an higher instruction sets, but none (or very few?) of those instructions are used, so to avoid building useless stuff

@giordano giordano force-pushed the mg/marcs-wrappers branch 2 times, most recently from af3226b to f80d2d9 Compare June 15, 2020 23:26
@giordano
Copy link
Member Author

First package built for multiple architectures: https://github.com/giordano/Darknet_jll.jl

@giordano giordano force-pushed the mg/marcs-wrappers branch 3 times, most recently from 28bd1af to 3e684c7 Compare June 17, 2020 00:26
@Keno Keno mentioned this pull request Jun 17, 2020
Closed
@giordano giordano marked this pull request as ready for review June 18, 2020 00:41
staticfloat
staticfloat requested changes Jun 18, 2020
View reviewed changes
src/Platforms.jl Outdated Show resolved Hide resolved
src/Platforms.jl Show resolved Hide resolved
src/Platforms.jl Show resolved Hide resolved
src/Rootfs.jl Show resolved Hide resolved
src/Rootfs.jl Outdated
ExtendedPlatform(Linux(:aarch64, libc=:glibc); march="carmel")
ExtendedPlatform(Linux(:aarch64, libc=:glibc); march="thunderx2")

julia> expand_marchs(Windows(:i686))
1-element Array{Windows,1}:
Windows(:i686)
```
"""
function expand_marchs(p::Platform)
Copy link
Member

Choose a reason for hiding this comment

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

I'm not 100% sold on expand_marchs() as a name.

  • expand_microarchitectures() is very clear.
  • expand_marches() sounds better, but reads worse.
  • expand_cpu_types() is shorter than the first one, but still gets the idea across.

What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, happy to change the name. I went with expand_microarchitectures, we already have very verbose expand_* functions 😀

src/Runner.jl Outdated
@@ -23,13 +23,27 @@ function proc_family(p::Platform)
end
end

function supported_marchs(p::Platform)
Copy link
Member

Choose a reason for hiding this comment

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

This should follow the naming of expand_marchs() above, whatever we decide upon.

Copy link
Member Author

Choose a reason for hiding this comment

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

Renamed to supported_microarchitectures. I've also moved it to src/Rootfs.jl, as this function now is used only by expand_microarchitectures() (in previous revisions it was used elsewhere, but this is now unnecessary)

src/Runner.jl Outdated Show resolved Hide resolved
src/Runner.jl Outdated Show resolved Hide resolved
src/Runner.jl Outdated Show resolved Hide resolved
src/Runner.jl Show resolved Hide resolved
@giordano giordano mentioned this pull request Jun 18, 2020
Open
giordano and others added 2 commits June 18, 2020 22:16
giordano
giordano commented Jun 18, 2020
View reviewed changes
Copy link
Member Author

@giordano giordano 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 I addressed all comments, thanks for the review!

src/Platforms.jl Outdated Show resolved Hide resolved
src/Platforms.jl Show resolved Hide resolved
src/Runner.jl Show resolved Hide resolved
src/Rootfs.jl Outdated
ExtendedPlatform(Linux(:aarch64, libc=:glibc); march="carmel")
ExtendedPlatform(Linux(:aarch64, libc=:glibc); march="thunderx2")

julia> expand_marchs(Windows(:i686))
1-element Array{Windows,1}:
Windows(:i686)
```
"""
function expand_marchs(p::Platform)
Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, happy to change the name. I went with expand_microarchitectures, we already have very verbose expand_* functions 😀

src/Runner.jl Outdated
@@ -23,13 +23,27 @@ function proc_family(p::Platform)
end
end

function supported_marchs(p::Platform)
Copy link
Member Author

Choose a reason for hiding this comment

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

Renamed to supported_microarchitectures. I've also moved it to src/Rootfs.jl, as this function now is used only by expand_microarchitectures() (in previous revisions it was used elsewhere, but this is now unnecessary)

@giordano giordano merged commit 24d00a1 into JuliaPackaging:master Jun 18, 2020
@giordano giordano deleted the mg/marcs-wrappers branch June 18, 2020 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@staticfloat staticfloat staticfloat approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@giordano @staticfloat