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

ICEs with incremental compilation building glium #34991

Closed
nikomatsakis opened this issue Jul 23, 2016 · 12 comments
Closed

ICEs with incremental compilation building glium #34991

nikomatsakis opened this issue Jul 23, 2016 · 12 comments
Labels
A-incr-comp Area: Incremental compilation T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

@jonas-schievink writes in #34956:

I just tried to build glium with this and I'm hitting multiple ICEs about local/foreign items:
https://gist.github.com/jonas-schievink/536a1ad7aa065193e4954242c9885cbd
(I've set RUSTFLAGS=-Zincremental=target/INCREMENTAL before running cargo)

@nikomatsakis nikomatsakis added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-incr-comp Area: Incremental compilation labels Jul 23, 2016
@nikomatsakis nikomatsakis added this to the Incremental compilation alpha milestone Jul 23, 2016
@jonas-schievink
Copy link
Contributor

This happens on stable too, btw

@michaelwoerister
Copy link
Member

This happens on stable too, btw

You mean, if you specify -Zincremental on stable?

@jonas-schievink
Copy link
Contributor

@michaelwoerister Yes

@nikomatsakis
Copy link
Contributor Author

So I did some digging. There are a few bugs I think.

One of them occurs is you run with -j1 -v, in which case you get an error in the SVH code at src/librustc_incremental/calculate_svh.rs:94, where we call expect_item, but sometimes the def-id corresponds to a foreign-item.

There seems to be a distinct bug that occurs with the default amount of parallelism which (I think) must relate to reading the wrong files or something like that. Still investigating.

@nikomatsakis
Copy link
Contributor Author

Fixing the problem with expect_item was easy. I am now investigating the second failure. Seems to be that when processing xml-rs we create an invalid read from a foreign HIR node, labeled as <std::vec::Vec<T> as std::ops::Drop>. I'm not 100% sure yet where this occurs but I guess sometime in trans?

@nikomatsakis
Copy link
Contributor Author

OK, the other problem goes away with -Z orbit, and is due to how "old trans" handles inlined functions.

@michaelwoerister
Copy link
Member

Have you tried turning backtraces on? That should give you idea where it occurs.

On July 26, 2016 8:44:31 PM GMT+02:00, Niko Matsakis notifications@github.com wrote:

Fixing the problem with expect_item was easy. I am now investigating
the second failure. Seems to be that when processing xml-rs we create
an invalid read from a foreign HIR node, labeled as <std::vec::Vec<T> as std::ops::Drop>. I'm not 100% sure yet where this occurs but I
guess sometime in trans?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#34991 (comment)

@nikomatsakis
Copy link
Contributor Author

Actually, I still get an ICE even with -Z orbit, but it occurs later. It seems to all be connected to register_reads, which seems a bit suspicious to me now that I look at it a bit more closely. I'm actually not sure if it's even needed, to be honest. It seems like the "trans item" is just a def-id and substs, so I would expect we should just start a suitable task and whatever reads are needed will be generated automatically (for example, if we use the def-id to fetch the MIR for an item, that should generate a read from the table where the MIR is stored; if the MIR has to be loaded from meta-data, it should generate a read from the metadata, etc).

@nikomatsakis
Copy link
Contributor Author

Maybe @michaelwoerister we should touch base on the whole scheme here to make sure we've got it all setup right? Doesn't feel like trans needs to be particularly special.

@nikomatsakis
Copy link
Contributor Author

Making progress here. I've eliminated a few sources of ICEs, but still have a problem left. I may post an intermediate PR, or may press on. :)

@nikomatsakis
Copy link
Contributor Author

OK, so I have all the ICEs fixed. I'm encountering a bit of trouble because cargo is using the same crate name / disambiguator (build-script-build / ``) for every build.rs file, so unless you use-j1 we run into #32754.

Only problem is that I have no test cases: most of these ICEs arise from multi-crate scenarios and I didn't bother to reduce them. Probably should do so before opening a PR.

@nikomatsakis
Copy link
Contributor Author

One other problem: running, then using cargo clean and running again, doesn't seem to save much time. :( Will have to see what's causing it not to reuse results. Probably better to start with a very simple crate though. :)

bors added a commit that referenced this issue Aug 9, 2016
Address ICEs running w/ incremental compilation and building glium

Fixes for various ICEs I encountered trying to build glium with incremental compilation enabled. Building glium now works. Of the 4 ICEs, I have test cases for 3 of them -- I didn't isolate a test for the last commit and kind of want to go do other things -- most notably, figuring out why incremental isn't saving much *effort*.

But if it seems worthwhile and I can come back and try to narrow down the problem.

r? @michaelwoerister

Fixes #34991
Fixes #32015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-incr-comp Area: Incremental compilation T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants