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

Core.Compiler removed required methods in 1.5 < #71

Open
femtomc opened this issue Jun 18, 2020 · 11 comments
Open

Core.Compiler removed required methods in 1.5 < #71

femtomc opened this issue Jun 18, 2020 · 11 comments

Comments

@femtomc
Copy link

femtomc commented Jun 18, 2020

ERROR: LoadError: UndefVarError: just_construct_ssa not defined

when using the package in 1.5.

In 1.6, there's also an issue with Params moved out of Core.Compiler (to make room for AbstractInterpreter.

Just throwing it up here so it will be on the radar.

@MikeInnes
Copy link
Member

We've been just_construct_ssa-free for a while and the latest release passes tests on 1.5. I also just removed a last vestige of typed IR in 93fa60d.

A new problem on 1.6 is that the Base IRCode constructor has changed. We only touch Base's IR format when converting IR to CodeInfo. The options are to fix that constructor, assuming Base IR hasn't changed too much, or to just rewrite that conversion ourselves, which is a little harder but more of a long-term fix.

A while back we wrote our own CodeInfo -> IR conversion and that's proved pretty robust (we haven't had to touch it since).

@femtomc
Copy link
Author

femtomc commented Jun 18, 2020

Looks like I didn't update my dependencies before precompiling - apologies for the non-issue.

If I'm understanding correctly on the second point, the issue is now going back from IR -> CodeInfo. I'm guessing this functionality is used in evalir and func? So that you can can lower to the IRTools IR, then return to CodeInfo and release to the normal pipeline.

I'm in no position to say, but the latter option seems optimal - you basically de-couple IRTools from Base changes permanently.

@IanButterworth
Copy link
Contributor

Is there a fix/workaround for Params not defined in julia master?

@IanButterworth
Copy link
Contributor

It'd be good to get Flux working on julia master given 1.5 is now released.

One reason is so that invalidations can be investigated with the new tooling

@femtomc
Copy link
Author

femtomc commented Aug 31, 2020

@ianshmean do you mean Zygote? I didn’t think Flux had any dependency else through Zygote?

@IanButterworth
Copy link
Contributor

Indeed. But ultimately it blocks Flux, so I thought I'd refer to that given it's the main package in the org.

@femtomc
Copy link
Author

femtomc commented Aug 31, 2020

Sure - generically there’s two directions here. One is to make Zygote compatible with > 1.6 - that means we need to fix these issues.

The second direction is to provide a backend for Flux which uses the new AD system in development. I’m guessing that new AD will become available as 1.6 releases.

In the first case, I’m not sure I can be of much help. I’m not a contributor here, and I don’t have the bandwidth currently to dig in.

@IanButterworth
Copy link
Contributor

I was unaware that Flux was undergoing a big change like that. Is there a particular issue to track?

@femtomc
Copy link
Author

femtomc commented Aug 31, 2020

It’s not Flux in particular. It’s a part of new compiler infrastructure. You can follow the work here: https://github.com/Keno/Compiler3.jl

@IanButterworth
Copy link
Contributor

Ok. So if I understand correctly, you're saying that that is due to be merged before 1.6, thus fixing Flux will probably wait until after that merges?

@femtomc
Copy link
Author

femtomc commented Aug 31, 2020

I don't have information about what's going on here. This is just my pure speculation. But it is known that new AD is coming (and Keno is working on it, as a part of this work which I mentioned above). And I'm guessing that Flux will have some effort associated with using the new AD.

Regardless, it would be good to fix these issues anyways so I totally agree with you.

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

No branches or pull requests

3 participants