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

Get tests to pass when running Julia with --compiled-modules=no. #114

Merged
merged 2 commits into from
Feb 2, 2021

Conversation

chriselrod
Copy link
Collaborator

This fixes JuliaSIMD/LoopVectorization.jl#192 (comment) for me: Trixi's tests pass, as do ArrayInterface's after starting Julia with --compiled-modules=no.

Normally, all functions in a module have the same world age, so the order in which generated functions are defined with respect to everything else is irrelevant. But not with --compiled-modules=no, which causes world age errors whenever an @generated function calls a method defined later.

I addressed this the lazy way, but just searching for all the @generated functions calling non-base methods and sticking them in a file at the end.

We could certainly do a lot better organizationally.

@Tokazama I'm not sure why it isn't letting me request you as a reviewer.

@codecov
Copy link

codecov bot commented Feb 2, 2021

Codecov Report

Merging #114 (c82c7c5) into master (db43f3f) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #114   +/-   ##
=======================================
  Coverage   84.26%   84.26%           
=======================================
  Files           8        8           
  Lines        1506     1506           
=======================================
  Hits         1269     1269           
  Misses        237      237           
Impacted Files Coverage Δ
src/ArrayInterface.jl 84.81% <ø> (ø)
src/stridelayout.jl 76.38% <ø> (-0.75%) ⬇️
src/dimensions.jl 89.59% <100.00%> (+0.43%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update db43f3f...44b06f2. Read the comment docs.

@Tokazama
Copy link
Member

Tokazama commented Feb 2, 2021

We could certainly do a lot better organizationally

Agreed. Did you have any suggestions? I've been trying to move away from having one gigantic file but I probably complicated things in the process

@chriselrod
Copy link
Collaborator Author

Agreed. Did you have any suggestions? I've been trying to move away from having one gigantic file but I probably complicated things in the process

I think breaking it up into indexing/dimensions/etc is fine, but breaking things up into "is it a @generated function", which this PR did, is not. Except that it fixes world age problems introduced by using --compiled-modules=no.

However, I now deleted generatedfuncs.jl, moving most of the code back, with a much more minor reordering to make sure we don't get world age issues under --compiled-modules=no. @generated definitions should all be following the definitions they're using now, or at least all of them covered by our unit tests.

So this PR is in much better shape/much less objectionable now. I'll merge and make a new release fairly soon.

@chriselrod chriselrod merged commit 854a4b8 into master Feb 2, 2021
@DilumAluthge DilumAluthge deleted the generatedfuncslast branch February 2, 2021 20:42
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.

World age error from LoopVectorization v0.10.0 with Tullio and MPI on Julia v1.5.3
2 participants