Skip to content

Remove deps folder and check AES-NI in __init__(). #11

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 4 commits into from
Mar 2, 2022
Merged

Remove deps folder and check AES-NI in __init__(). #11

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7b96402
Remove `deps` folder and check AES-NI in `__init__()`.
sunoru Feb 27, 2022
46ede96
Use `success` to run the process.
sunoru Feb 27, 2022
6be706e
Test.
sunoru Mar 2, 2022
ef42ab2
No x86 on MacOS.
sunoru Mar 2, 2022
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
7 changes: 5 additions & 2 deletions .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,12 @@ jobs:
runs-on: ${{ matrix.os }}
strategy:
matrix:
julia-version: ["1.5", "1.6", nightly]
julia-arch: [x64]
julia-version: ["1.5", "1.7", nightly]
julia-arch: [x64, x86]
os: [ubuntu-latest, windows-latest, macOS-latest]
exclude:
- os: macOS-latest
julia-arch: x86

steps:
- uses: actions/checkout@v2
Expand Down
24 changes: 0 additions & 24 deletions deps/build.jl
View file
Open in desktop

This file was deleted.

5 changes: 0 additions & 5 deletions deps/has_aesni.jl
View file
Open in desktop

This file was deleted.

41 changes: 32 additions & 9 deletions src/Random123.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ module Random123

using RandomNumbers

export R123_USE_AESNI, set_counter!
export set_counter!
include("common.jl")

export Threefry2x, Threefry4x
Expand All @@ -27,15 +27,38 @@ include("threefry.jl")
export Philox2x, Philox4x
include("philox.jl")

if R123_USE_AESNI
include("aesni_common.jl")
export AESNI1x, AESNI4x
include("aesni.jl")
"True when AES-NI has been enabled."
const R123_USE_AESNI = Ref(false)
export R123_USE_AESNI
export AESNI1x, AESNI4x
export ARS1x, ARS4x
include("./aesni/module.jl")

export ARS1x, ARS4x
include("ars.jl")
else
@warn "AES-NI is not enabled, so AESNI and ARS are not available."
function __init__()

R123_USE_AESNI[] = try
cmd = Base.julia_cmd()
Copy link

@maleadt maleadt Mar 3, 2022

Choose a reason for hiding this comment

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

__init__ is for simple initialization; spawning a Julia process is not that and should not happen there. Either do this globally, and embed the use of AES-NI into the precompilation image (which should be safe, as that will not change across sessions) or make accesses to related functions lazy and have them initialize the bool upon first use.

Copy link
Member Author

@sunoru sunoru Mar 3, 2022

Choose a reason for hiding this comment

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

What do you mean by doing it globally? We used to do this in deps/build.jl but then decided that checking it every time the package is loaded may be better.

Copy link
Member Author

@sunoru sunoru Mar 3, 2022

Choose a reason for hiding this comment

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

#3 (comment)

Copy link

@maleadt maleadt Mar 3, 2022

Choose a reason for hiding this comment

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

Once, globally. const use_aesni = .... That'll be embedded in your precompiled package and not be evaluated every time you import the package. It's still better than deps/build.jl, which will only fire once when you install the package, since precompilation images are (should be) regenerated once the environment changes.

But again, for full flexibility, use a dynamic getter (a function that returns whether to use AES-NI, computes that lazily, and caches it per session). Look at how Flux.jl decides whether to use GPU support or not, for inspiration.

Copy link
Member Author

@sunoru sunoru Mar 3, 2022

Choose a reason for hiding this comment

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

Thank you so much. This is really helpful! I'm gonna have a look when I have time.

push!(
cmd.exec, "-e",
"const __m128i = NTuple{2, VecElement{UInt64}};" *
"@assert ccall(\"llvm.x86.aesni.aeskeygenassist\", " *
"llvmcall, __m128i, (__m128i, UInt8), " *
"__m128i((0x0123456789123450, 0x9876543210987654)), 0x1) ≡ " *
"__m128i((0x857c266f7c266e85, 0x2346382146382023))"
)
success(cmd)
catch e
@show e
false
end

if R123_USE_AESNI[]
@eval using ._AESNIModule
Copy link

@maleadt maleadt Mar 3, 2022

Choose a reason for hiding this comment

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

This is not allowed, and triggers JuliaGPU/CUDA.jl#1422.

Copy link
Member Author

@sunoru sunoru Mar 3, 2022

Choose a reason for hiding this comment

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

I have seen that. I realized @eval may not be necessary. Will look into this later..

Copy link

@maleadt maleadt Mar 3, 2022

Choose a reason for hiding this comment

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

You cannot do a using at non-toplevel scope. So this will never work from __init__.

Copy link
Member Author

@sunoru sunoru Mar 3, 2022

Choose a reason for hiding this comment

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

Oh I see... Is there a way to lazy load this? Like what Requires.jl does

Copy link

@maleadt maleadt Mar 3, 2022

Choose a reason for hiding this comment

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

It's possible, one part of the trick is not to do so while precompiling, https://github.com/JuliaPackaging/Requires.jl/blob/542a641b633fa38a5950c2b1dbb90861967c7015/src/require.jl#L97-L105.

else
@warn "AES-NI is not enabled, so AESNI and ARS are not available."
end

nothing
end

end
0 src/aesni.jl → src/aesni/aesni.jl
View file
Open in desktop
File renamed without changes.
0 src/ars.jl → src/aesni/ars.jl
View file
Open in desktop
File renamed without changes.
3 changes: 3 additions & 0 deletions src/aesni_common.jl → src/aesni/common.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
using Base: llvmcall
import Base.(+)

using ..Random123: R123Generator1x, R123Generator4x
import ..Random123: random123_r, set_counter!

const __m128i = NTuple{2, VecElement{UInt64}}
Base.convert(::Type{__m128i}, x::UInt128) = unsafe_load(Ptr{__m128i}(pointer_from_objref(Ref(x))))
Base.convert(::Type{UInt128}, x::__m128i) = unsafe_load(Ptr{UInt128}(pointer_from_objref(Ref(x))))
Expand Down
14 changes: 14 additions & 0 deletions src/aesni/module.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
module _AESNIModule

using ..Random123

export __m128i, AESNIKey
include("common.jl")

export AESNI1x, AESNI4x
include("aesni.jl")

export ARS1x, ARS4x
include("ars.jl")

end
6 changes: 0 additions & 6 deletions src/common.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,6 @@ import Libdl
import Random: rand, seed!
import RandomNumbers: AbstractRNG

const _dep_dir = joinpath(dirname(@__FILE__), "../deps/")
include_dependency(joinpath(_dep_dir, "build.log"))

"True when AES-NI has been enabled."
const R123_USE_AESNI = isfile(joinpath(_dep_dir, "aes-ni"))

const R123Array1x{T<:Union{UInt128}} = NTuple{1, T}
const R123Array2x{T<:Union{UInt32, UInt64}} = NTuple{2, T}
const R123Array4x{T<:Union{UInt32, UInt64}} = NTuple{4, T}
Expand Down
2 changes: 1 addition & 1 deletion test/aesni.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
if R123_USE_AESNI
if R123_USE_AESNI[]

import Random: seed!
using Test: @test
Expand Down
2 changes: 1 addition & 1 deletion test/ars.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
if R123_USE_AESNI
if R123_USE_AESNI[]

import Random: seed!
using Test: @test
Expand Down