These are chat archives for dropbox/pyston

17th
Aug 2015
Sun
@Daetalus
Aug 17 2015 02:53

Seems CPython have some minor naming issues. For example: floatinfo_desc, I think it should be just like float_info_desc, version_info_desc. And Long_InfoType should be LongInfoType? Like VersionInfoType and FloatInfoType?

PS: It's just internal identifiers, not change the public API.

lanyuesl
@lanyuesl
Aug 17 2015 03:48
I am not sure but I wander that if pyston use ast.py to generate AST of the python source code?
Sun
@Daetalus
Aug 17 2015 03:49
Nope, seems use @vinzenz 's libpypa to generate AST.
lanyuesl
@lanyuesl
Aug 17 2015 03:52
Is there any way to see the AST generate by pyston?
Sun
@Daetalus
Aug 17 2015 03:54
Use libpypa could check the generated AST.
But not from Pyston, just use CPython run some libpypa commands.
lanyuesl
@lanyuesl
Aug 17 2015 04:08
Thank you. And also, I think pyston translate the AST to the LLVM IR(or something else, witch stored in BasicBlock). However, I don't know why but, I can not break at the function 'emitBBs' with gdb.
I want to see the LLVM IR generate from python code. Is there any suggestion?
Kevin Modzelewski
@kmod
Aug 17 2015 04:27
you can try '-nvv' to increase the verbosity to the point that it includes the llvm IR
you probably want to run it as something like PYTHONIOENCODING=utf8 ./pyston_dbg -Snvv test.py
the extra arguments tell Pyston to not run the standard startup code which would otherwise clutter up the log
I'm not sure we print out the initial AST ever; we usually convert it pretty quickly to a CFG form (our ~bytecode)
which I think will get printed with -vv
Sun
@Daetalus
Aug 17 2015 06:16

Hi @kmod In CPython, long use an array to store digit. And according the comment in CPython,(CPython/Include/longintrepr.h). They use PYLONG_BITS_IN_DIGIT to determine the sys.long_info. Like this: sys.long_info(bits_per_digit=30, sizeof_digit=4).

But Pyston wrap GMP to implement long object. So doesn't need these parameters. So can we hard code the bits_per_digit and sizeof_digit for now?

For long term. Maybe we should just use CPython long implementation? For support different platfrom in future?

Kevin Modzelewski
@kmod
Aug 17 2015 06:38
hmm maybe we could
*could switch to using CPython's long impementation
GMP is much faster though
we've run into some other minor cases where we've had compatibility issues with it (at the C API level)
Sun
@Daetalus
Aug 17 2015 06:39
Ok. Just for now. Could we hard code bits_per_digit and sizeof_digit for now?
Kevin Modzelewski
@kmod
Aug 17 2015 06:39
yeah, maybe 0/0?
I think it would be better to not provide sys.long_info
if there's nothing reasonable that we could give for it
Sun
@Daetalus
Aug 17 2015 06:40
Emm, 0 may cause the long test failed? I didn't check it carefully.
test_long need to use it.
Indeed. Not provide long_info is reasonable.
And how to support long on Windows platform in future? Still use GMP?
Kevin Modzelewski
@kmod
Aug 17 2015 06:41
it looks like they only use it to generate test cases that they think will be interesting
hmm I don't know
Sun
@Daetalus
Aug 17 2015 06:42
Yes. So hard code the long_info in test case. And not provide long_info for now?
Kevin Modzelewski
@kmod
Aug 17 2015 06:43
sounds good :)
yeah I don't know what we'll do for windows; maybe we could use the cpython implementation there
Sun
@Daetalus
Aug 17 2015 06:44
ok.
A minor thing. In Pyston/from_cpython/Include, somehow the PY_MICRO_VERSION was set to 7. Does Pyston target to 2.7.6?
And can I change PY_RELEASE_LEVEL from PY_RELEASE_LEVEL_FINAL to PY_RELEASE_LEVEL_ALPHA in same file?
Sorry, In Pyston/from_cpython/Include/patchlevel.h
Kevin Modzelewski
@kmod
Aug 17 2015 06:57
I think when we started copying things in, we did it from the 2.7.7 release since that was the latest at the time
I'm not that familiar with what has changed during the 2.7.x series
Sun
@Daetalus
Aug 17 2015 06:59
Ok, leave the PY_MICRO_VERSION. But what about PY_RELEASE_LEVEL?
Vinzenz Feenstra
@vinzenz
Aug 17 2015 07:04
@kmod about the kwargs dict being empty: I seem to fail to do the checks correctly I have been trying friday to fix that already, but somehow it keeps failing on me. I'll see what I can do about that.
Kevin Modzelewski
@kmod
Aug 17 2015 07:04
I hope it doesn't matter
I'm not sure what people would want to do with that field
I tried searching github and the only results look like people who copied in parts of cpython
unless anyone feels strongly, I think we should just leave it as-is for now, and we can take another look when we start doing more formalized releases
@vinzenz ok cool :)
glad to have you back again
Sun
@Daetalus
Aug 17 2015 07:07
I going to check Python release notes.
Ok. Git it.
Sun
@Daetalus
Aug 17 2015 07:22
^Got it
Travis Hance
@tjhance
Aug 17 2015 08:12
So I added the ability to do a guard jump and then move the args into place, and then call the slowpath. No impact on overall perf (since it seems like the args are usually in place anyway), but this feature is necessary for the callattr slowpath stuff. I also have the slowpath stuff in another branch (the buggy PR). Seems like it should just be a matter of merging the two (which won't be too easy)
Hopefully this will also be a stepping stone to supporting branching first class in the rewriter, although substantially more work is required for that, probably
Marius Wachtler
@undingen
Aug 17 2015 09:15
:)
I will try to come up with a inlining prototype during the hack week, I just created the project.
Vinzenz Feenstra
@vinzenz
Aug 17 2015 09:38
it'd be good if builtin functions like id won't say their name is just 'builtin' during the call, but actually have their names displayed. Since they're registered as such that should be possible, no?
Vinzenz Feenstra
@vinzenz
Aug 17 2015 10:15
@kmod Got it now and tests passed. I fixed a few more things though than just that kwargs thing. I am actually trying to get the extcall test working
tests passed on Travis already
Kevin Modzelewski
@kmod
Aug 17 2015 10:18
re the "builtin" thing, I think it's mostly just a matter of making sure that we pass the function name when we create the function object
which we don't usually do right now
but yeah it's annoying that it doesn't give you a nice error message
or repr()
Vinzenz Feenstra
@vinzenz
Aug 17 2015 10:50
@kmod i am right looking into that it will use the name when present.
I think I got it already
Also this is part of the extcall test failures
Vinzenz Feenstra
@vinzenz
Aug 17 2015 11:08
@kmod it also looks like that pyston does conceptually something different than CPython. Namely that it seems to evaluate first that a call target is callable, before it evaluates the parameters. From the test results of test_extcall you can see that we're failing on 'NoneType' object is not callable where CPython fails on the parameters passed
Marius Wachtler
@undingen
Aug 17 2015 12:38
I have a patch which adds a few __repr__ attributes and gives better name output. I will upload it shortly
Travis Hance
@tjhance
Aug 17 2015 15:37
Hm, I'm pretty sure we evaluate the arguments first
Sun
@Daetalus
Aug 17 2015 16:19
It'sseems just the expception handle order issue.
Kevin Modzelewski
@kmod
Aug 17 2015 19:47
hmm it could also be an issue during irgen
ie, sometimes we detect that something will always fail and then directly emit a raise
and we might forget to evaluate the arguments first
Kevin Modzelewski
@kmod
Aug 17 2015 20:02
@tjhance sounds cool about the rewriter :)
I was thinking, maybe one idea is that we could create sub-rewriters to handle this
Travis Hance
@tjhance
Aug 17 2015 20:02
maybe it needs a concept of basic block
Kevin Modzelewski
@kmod
Aug 17 2015 20:02
ie rather than having one rewriter handle all of the restoring, we could create a new rewriter object
Travis Hance
@tjhance
Aug 17 2015 20:03
yeah it’s annoying cause it’s really stateful right now
like the uses thing
Kevin Modzelewski
@kmod
Aug 17 2015 20:04
I think in the cases we've run into at least so far, it's usually that there's some discrete operation we want to do and want to make sure gets rewritten
Sun
@Daetalus
Aug 17 2015 20:04
Hey, would your mind to give a brief introduction about what does rewrite did, if you have time? I just want to get more familiar with Pyston code base.
Kevin Modzelewski
@kmod
Aug 17 2015 20:04
but we can't rewrite it as part of the overall rewrite
also, it would let us keep the notion that the slowpath is for the top-level rewrite
ie we're going to run into a situation where we're deep inside a rewrite and we realize that we want to use this sort of functionality
we can't necessarily say "ok the slowpath is foo() from now on"
since we don't know if there's other stuff that we would want to run after the current function ends
but we could say out_rtn = fooWithRewriter(createSubRewriter(rewriter))
Travis Hance
@tjhance
Aug 17 2015 20:06
hm so for callattr that’s basically what I do (I declare “the slowpath is foo() from now on”)
oh
hmm
that is a good point
Kevin Modzelewski
@kmod
Aug 17 2015 20:08
@Daetalus our rewriter is basically a mini tracing JIT
Travis Hance
@tjhance
Aug 17 2015 20:08
I wonder why I didn’t run into that problem yet
Kevin Modzelewski
@kmod
Aug 17 2015 20:08
that traces out a single bytecode at a time
Travis Hance
@tjhance
Aug 17 2015 20:08
I bet callattrInternal is only ever used as the last step
hmm
Sun
@Daetalus
Aug 17 2015 20:08
Ok, thanks!
Kevin Modzelewski
@kmod
Aug 17 2015 20:09
we thread through some state ("rewrite_args") through functions
and the functions have to know how to "rewrite" (ie emit a trace for) themselves
the name "rewriter" comes from the fact that it originally was just for rewriting simple inline caches such as property lookups
I guess it still does do a "rewrite", it rewrites a call to a slowpath to the trace for it
@tjhance yeah maybe if there's anything else after the callattr the rewrite will fail anyway, so we don't run into those cases?
Travis Hance
@tjhance
Aug 17 2015 20:10
yeah if there’s anything else after the callattr that requires guards
it will definitely fail
Sun
@Daetalus
Aug 17 2015 21:14
Thanks @kmod
Oh....
I think I should just let you merge #846 first...
Kevin Modzelewski
@kmod
Aug 17 2015 21:16
oh, either way is fine
sorry, wasn't sure when you would get to it, so I just created #846 to make sure it gets included at some point
since it's a bug on master
Sun
@Daetalus
Aug 17 2015 21:17
That's OK
Would you mind to restart the CI after you merge #846, please? Or should I rebase it then commit again?
Kevin Modzelewski
@kmod
Aug 17 2015 21:21
yeah, you'll need to recommit
restarting the CI will just re-do the test using the same merge point
Sun
@Daetalus
Aug 17 2015 21:21
Ok. Got it.
Sun
@Daetalus
Aug 17 2015 23:34
@kmod I just found #873 and #830 plus a minor change could also fix test_math. So I will update it manually.
#837
Kevin Modzelewski
@kmod
Aug 17 2015 23:37
oh cool :)