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

Fpcast removal #1677

Closed
wants to merge 71 commits into from
Closed

Fpcast removal #1677

wants to merge 71 commits into from

Conversation

joemarshall
Copy link
Contributor

@joemarshall joemarshall commented Jun 30, 2021

This is a first attempt at removing fpcast from the build. Quite a lot of tests pass now, including numpy, scipy.

Closes #1577

@joemarshall
Copy link
Contributor Author

CI seemed to fail on lots of things here, despite working fine on my build machine. Lots of timeouts...

@hoodmane
Copy link
Member

hoodmane commented Jun 30, 2021

Well at least it passed on Firefox (the run_webworker and reentrant_error tests regularly fail on main). It must be crashing chrome. I don't have any good way of understanding what is going on when code works on one browser and totally breaks the other though.

One recent problem I hit is that chrome is more delicate about pointer alignment than firefox. The following code crashes chrome consistently (though it is also noticeably slow on firefox):

EM_JS(void, my_func, (), {
	  let addr = _malloc(8);
	  let byte_ptr = ptr1 + 3;
	  let ptr_ptr = addr + 4;
	  HEAP8[byte_ptr] = 7;
	  HEAPU32[ptr_ptr/4] = byte_ptr;
	  _print_byte_ptrptr(ptr_ptr);
});

void print_byte_ptrptr(char **p){
	printf("byte is %x\n", **p);
}

@hoodmane
Copy link
Member

Oops didn't mean to push here, sorry.

@rth
Copy link
Member

rth commented Nov 13, 2021

I synced with main after patches were merged in #1708. There were still a few failures even with the latest CI setup that look genuine to me though.

@hoodmane
Copy link
Member

hoodmane commented Nov 13, 2021

Well one problem is that libffi needs to be updated. I think we can just remove one of the compile flags from libffi since my port uses #ifdef to handle different fpcast options.

@hoodmane
Copy link
Member

hoodmane commented Dec 2, 2021

@rth On circleci I keep getting:

make[1]: *** [Makefile:2020: Modules/unicodedata.o] Killed

each time in the same place, at around 11 minutes and 20 seconds +/- a bit. Sometimes it doesn't happen. Related to #2013?

@hoodmane
Copy link
Member

hoodmane commented Dec 2, 2021

@rth Also, is there a way to manually flush all ccaches when merging this? I changed the compiler, so all caches need to be invalidated, but I think the cache invalidation mechanism only looks for changes in the source, not for changes in the compiler.

@hoodmane
Copy link
Member

hoodmane commented Dec 2, 2021

Oh I think the problem might be that it uses up too much ram loading the AST for unicodedata.o. The AST json dump unicodedata.c.ast is a gigabyte! Maybe I should check whether the ast is super big and if so skip the patching step...

@hoodmane
Copy link
Member

hoodmane commented Dec 2, 2021

Another option would be to use a compiled language to parse the ast or something. Sounds annoying though.

@hoodmane
Copy link
Member

hoodmane commented Dec 2, 2021

unicodedata.c has 8000 lines of static data. The ast really blows that up by recording a bunch of location information for each 0.

@rth
Copy link
Member

rth commented Dec 2, 2021

Also, is there a way to manually flush all ccaches when merging this?

Normally it hashes Makefile.env for the cache key, so you should be OK. Otherwise you can bump the date in

- -core-{{ checksum "Makefile.envs" }}-{{ checksum "cpython/Makefile" }}-v20210910-
and similar lines manually.

The AST json dump unicodedata.c.ast is a gigabyte! Maybe I should check whether the ast is super big and if so skip the patching step...

Any way to process it with a generator without loading it all in memory?

@hoodmane
Copy link
Member

hoodmane commented Dec 2, 2021

Any way to process it with a generator without loading it all in memory?

Ironically processing the human-readable AST made this a lot easier. But the best approach is probably to use jq.

@hoodmane
Copy link
Member

hoodmane commented Dec 2, 2021

@hoodmane
Copy link
Member

hoodmane commented Dec 3, 2021

I think this is working now, but am having trouble with build timeouts.
But I just thought of a different approach to this which might be easier. Basically, we should just patch call trampolines into the Python interpreter in a few places rather than trying to fix the package source code.

@hoodmane
Copy link
Member

hoodmane commented Dec 5, 2021

Closed in favor of #2019.

@hoodmane hoodmane closed this Dec 5, 2021
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.

fpcast emulation tracing and removing
4 participants