[issue11549] Rewrite peephole to work on AST
Nick Coghlan ncogh...@gmail.com added the comment: Eugene raised the question of AST changes on python-dev [1] and the verdict was that so long as ast.__version__ is updated, AST clients will be able to cope with changes. Benjamin Peterson made some subsequent changes to the AST (bringing the AST for try and with statements more in line with the concrete syntax, allowing source-to-source transforms to retain the original code structure). This patch will probably need to be updated to be based on the latest version of the AST - I would be surprised if it applied cleanly to the current tip. [1] http://mail.python.org/pipermail/python-dev/2011-April/110399.html -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
anatoly techtonik techto...@gmail.com added the comment: Is there any tool to see how it works step-by-step. The whole stuff is extremely interesting, but I can't fit all the details of AST processing in my head. -- nosy: +techtonik ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Nick Coghlan ncogh...@gmail.com added the comment: Eugene: I suggest raising the question on python-dev. The answer potentially affects the PEP 380 patch as well (which adds a new attribute to the Yield node). Anatoly: If you just want to get a feel for the kind of AST various pieces of code produce, then the ast.dump function (along with using the ast.PyCF_ONLY_AST flag in compile) may be enough. You may also want to take a look at the AST - dot file conversion code in Dave Malcolm's patches on #10399. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Eugene Toder elto...@gmail.com added the comment: If we have to preserve backward compatibility of Python AST API, we can do this relatively easily (at the expense of some code complexity): * Add 'version' argument to compile() and ast.parse() with default value of 1 (old AST). Value 2 will correspond to the new AST. * Do not remove Num/Str/Bytes/Ellipsis Python classes. Make PyAST_obj2mod and PyAST_mod2obj do appropriate conversions when version is 1. * Possibly emit a PendingDeprecationWarning when version 1 is used with the goal of removing it in 3.5 Alternative implementation is to leave Num/Str/etc classes in C as well, and write visitors (similar to folding one) to convert AST between old and new forms. Does this sounds reasonable? Should this be posted to python-dev? Should I write a PEP (I'd need some help with this)? Are there any other big issues preventing this to be merged? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Raymond Hettinger raymond.hettin...@gmail.com added the comment: Eugene, I think you're doing great work here and would like to see you succeed. In the near term, I don't have time to participate, but don't let that stop you. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Nick Coghlan ncogh...@gmail.com added the comment: Finally got around to reviewing this (just a visual scan at this stage) - thanks for the effort. These are mostly big picture type comments, so I'm keeping them here rather than burying them amongst all the details in the code review tool. The effect that collapsing Num/Str/Bytes into a single Lit node type has on ast.literal_eval bothered me initially, but looking more closely, I think those changes will actually improve the function (string concatenation will now work, and errors like 'hello' - 'world' should give a more informative TypeError). (Bikeshed: We use Attribute rather than Attr for that node type, perhaps the full Literal name would be better, too) Lib/test/disutil.py should really be made a feature of the dis module itself, by creating an inner disassembly function that returns a string, then making the existing dis and disassembly functions print that string (i.e. similar to what I already did in making dis.show_code() a thin wrapper around the new dis.code_info() function in 3.2). In the absence of a better name, dis_to_str would do. Since the disassembly is interpreter specific, the new disassembly tests really shouldn't go directly in test_compile.py. A separate test_ast_optimiser file would be easier for alternate implementations to skip over. A less fragile testing strategy may also be to use the ast.PyCF_ONLY_AST flag and check the generated AST rather than the generated bytecode. I'd like to see a written explanation for the first few changes in test_peepholer.py. Are those cases no longer optimised? Are they optimised differently? Why did these test cases have to change? (The later changes in that file look OK, since they seem to just be updating tests to handle the more comprehensive optimisation) When you get around to rebasing the patch on 3.3 trunk, don't forget to drop any unneeded from __future__ imports. The generated code for the Lit node type looks wrong: it sets v to Py_None, then immediately checks to see if v is NULL again. Don't use string as a C type - use char * (and char ** instead of string *). There should be a new compiler flag to skip the AST optimisation step. A bunch of the compiler internal changes seem to make the basic flow of the generated assembly not match the incoming source code. It doesn't seem like a good idea to conflate these with the AST optimisation patch. If that means leaving the peepholer in to handle them for now, that's OK - it's fine to just descope the peepholer without removing it entirely. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Nick Coghlan ncogh...@gmail.com added the comment: I think the biggest thing to take out of my review is that I strongly encourage deferring the changes for 5(b) and 5(c). I like the basic idea of using a template-based approach to try to get rid of a lot of the boilerplate code currently needed for AST visitors. Providing a hook for optimisation in Python (as Dave Malcolm's approach does) is valuable as well, but I don't think the two ideas need to be mutually exclusive. As a more general policy question... where do we stand in regards to backwards compatibility of the AST? The ast module docs don't have any caveats to say that it may change between versions, but it obviously *can* change due to new language constructs (if nothing else). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Antoine Pitrou pit...@free.fr added the comment: As a more general policy question... where do we stand in regards to backwards compatibility of the AST? The ast module docs don't have any caveats to say that it may change between versions, but it obviously *can* change due to new language constructs (if nothing else). Yes, but can existing constructs produce a different structure from one Python version to another? It seems to me that the ast module is the recommended way to inspect the structure of Python code (much more so that bytecode or concrete syntax trees). Perhaps the optimizations can leave the initial ast intact? Perhaps with an additional API to get the optimized ast as well? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Georg Brandl ge...@python.org added the comment: I would provide this via another compile flag a la PyCF_ONLY_AST. If you give only this flag, you get the original AST. If you give (e.g.) PyCF_OPTIMIZED_AST, you get the resulting AST after the optimization stage (or the same, if optimization has been disabled). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Eugene Toder elto...@gmail.com added the comment: Thanks. string concatenation will now work, and errors like 'hello' - 'world' should give a more informative TypeError Yes, 'x'*5 works too. Bikeshed: We use Attribute rather than Attr for that node type, perhaps the full Literal name would be better Lit seemed more in line with Num, Str, BinOp etc. No reason it can't be changed, of course. Lib/test/disutil.py should really be made a feature of the dis module itself Agreed, but I didn't want to widen the scope of the patch. If this is something that can be reviewed quickly, I can make a change to dis. I'd add a keyword-only arg to dis and disassembly -- an output stream defaulting to stdout. dis_to_str then passes StringIO and returns the string. Sounds OK? Since the disassembly is interpreter specific, the new disassembly tests really shouldn't go directly in test_compile.py. A separate test_ast_optimiser file would be easier for alternate implementations to skip over. New tests in test_compiler are not for the AST pass, but for changes to compile.c. I can split them out, how about test_compiler_opts? I'd like to see a written explanation for the first few changes in test_peepholer.py Sure. 1) not x == 2 can be theoretically optimized to x != 2, while this test is for another optimization. not x is more robust. 2) Expression statement which is just a literal doesn't produce any code at all. This is now true for None/True/False as well. To preserve constants in the output I've put them in tuples. When you get around to rebasing the patch on 3.3 trunk, don't forget to drop any unneeded from __future__ imports. If you're referring to asdl_ct.py, that's actually an interesting question. asdl_ct.py is run by system installed python2, for obvious reasons. What is the policy here -- what is the minimum version of system python that should be sufficient to build python3? I tested my code on 2.6 and 3.1, and with __future__ it should work on 2.5 as well. Is this OK or should I drop 'with' so it runs on 2.4? The generated code for the Lit node type looks wrong: it sets v to Py_None, then immediately checks to see if v is NULL again. Right, comment in asdl_c.py says: # XXX: special hack for Lit. Lit value can be None and it # should be stored as Py_None, not as NULL. If there's a general agreement on Lit I can certainly clean this up. Don't use string as a C type - use char * (and char ** instead of string *). string is a typedef for PyObject that ASDL uses. I don't think I have a choice to not use it. Can you point to a specific place where char* could be used? There should be a new compiler flag to skip the AST optimisation step. There's already an 'optimizations level' flag. Maybe we should make it more meaningful rather than multiplying the number of flags? A bunch of the compiler internal changes seem to make the basic flow of the generated assembly not match the incoming source code. Can you give an example of what you mean? The changes are basically 1) standard way of handling conditions in simple compilers 2) peephole. It doesn't seem like a good idea to conflate these with the AST optimisation patch. If that means leaving the peepholer in to handle them for now, that's OK - it's fine to just descope the peepholer without removing it entirely. The reason why I think it makes sense to have this in a single change is testing. This allows to reuse all existing peephole tests. If I leave old peephole enabled there's no way to tell if my pass did something from disassembly. I can port tests to AST, but that seemed like more work than match old peepholer optimizations. Is there any opposition to doing simple optimizations on compiler structures? They seem a good fit for the job. In fact, if not for stack representation, I'd say that they are better IR for optimizer than AST. Also, can I get your opinion on making None/True/False into literals early on and getting rid of forbidden_name? Antoine, Georg -- I think Nick's question is not about AST changing after optimizations (this can indeed be a separate flag), but the structure of AST changing. E.g. collapsing of Num/Str/Bytes into Lit. Btw, if this is acceptable I'd make a couple more changes to make scope structure obvious from AST. This will allow auto-generating much of the symtable pass. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Eugene Toder elto...@gmail.com added the comment: and with __future__ it should work on 2.5 as well. Actually, seems that at least str.format is not in 2.5 as well. Still the question is should I make it run on 2.5 or 2.4 or is 2.6 OK (then __future__ can be removed). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Daniel Urban urban.dani...@gmail.com added the comment: not x == 2 can be theoretically optimized to x != 2, ... I don't think it can: class X: ... def __eq__(self, other): ... return True ... def __ne__(self, other): ... return True ... x = X() not x == 2 False x != 2 True -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Eugene Toder elto...@gmail.com added the comment: I don't think it can: That already doesn't work in dict and set (eq not consistent with hash), I don't think it's a big problem if that stops working in some other cases. Anyway, I said theoretically -- maybe after some conservative type inference. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Eugene Toder elto...@gmail.com added the comment: Also, to avoid any confusion -- currently my patch only runs AST optimizations before code generation, so compile() with ast.PyCF_ONLY_AST returns non-optimized AST. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Terry J. Reedy tjre...@udel.edu added the comment: While I would not be happy to use class X above, the 3.2 manual explicitly says There are no implied relationships among the comparison operators. The truth of x==y does not imply that x!=y is false. . -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Nick Coghlan ncogh...@gmail.com added the comment: OK, I missed the fact that the new optimisation pass isn't run under PyCF_ONLY_AST. However, as Eugene picked up, my concern is actually with the collapsing of Str/Num/Bytes/Ellipsis into the new Lit node, and the changes to the way None/True/False are handled. They're all changes that *make sense*, but would also likely cause a variety of AST manipulations to break. We definitely don't care when bytecode hacks break, but providing the ast module means that AST manipulation is officially supported. However, the reason I bring up new constructs, is the fact that new constructs may break AST manipulation passes, even if the old structures are left intact - the AST visitor may miss (or misinterpret) things because it doesn't understand the meaning of the new nodes. We may need to take this one back to python-dev (and get input from the other implementations as well). It's a fairly fundamental question when it comes to the structure of any changes. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Eugene Toder elto...@gmail.com added the comment: Is anyone looking or planing to look at the patch? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Terry J. Reedy tjre...@udel.edu added the comment: I suspect someone will sometime. There is bit of a backlog of older issues. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Eugene Toder elto...@gmail.com added the comment: AFAICT my patch has everything that #1346238 has, except BoolOps, which can be easily added (there's a TODO). I don't want to add any new code, though, until the current patch will get reviewed -- adding code will only make reviewing harder. #10399 looks interesting, I will take a look. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Changes by Georg Brandl ge...@python.org: -- nosy: +georg.brandl ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Changes by Skip Montanaro s...@pobox.com: -- nosy: -skip.montanaro ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Changes by Mark Dickinson dicki...@gmail.com: -- nosy: +mark.dickinson ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Changes by Nadeem Vawda nadeem.va...@gmail.com: -- nosy: +nvawda ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Terry J. Reedy tjre...@udel.edu added the comment: A couple of somewhat related issues: #10399 AST Optimization: inlining of function calls #1346238 A constant folding optimization pass for the AST Obviously, ast optimizers should work together and not duplicate. Nice to see increased attention. -- nosy: +terry.reedy ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Eugene Toder elto...@gmail.com added the comment: Any comments on the code so far or suggestions on how we should move forward? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Nick Coghlan ncogh...@gmail.com added the comment: I've been focusing on softer targets during the sprints - I'll dig into this once I'm back home and using my desktop machine again. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Changes by Alex Gaynor alex.gay...@gmail.com: -- nosy: +alex ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Eugene Toder elto...@gmail.com added the comment: I've updated patches on Rietveld with some small changes. This includes better code generation for boolops used outside of conditions and cleaned up optimize_jumps(). This is probably the last change before I get some feedback. Also, I forgot to mention yesterday, patches on Rietveld are vs. ab45c4d0b6ef -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Eugene Toder elto...@gmail.com added the comment: Just for fun I've run pystones. W/o my changes it averages to about 70k, with my changes to about 72.5k. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Changes by Daniel Urban urban.dani...@gmail.com: -- nosy: +durban ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Changes by Nick Coghlan ncogh...@gmail.com: -- nosy: +dmalcolm, ncoghlan ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Changes by Nick Coghlan ncogh...@gmail.com: -- nosy: +brett.cannon ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Skip Montanaro s...@pobox.com added the comment: I'm confused. Why aren't there review links? -- nosy: +skip.montanaro ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Eugene Toder elto...@gmail.com added the comment: Because I don't know how to make them. Any pointers? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Antoine Pitrou pit...@free.fr added the comment: Because I don't know how to make them. Any pointers? Martin is hacking on the tool these days... So it's no surprise it doesn't work perfectly yet ;) If you have a Google account you can upload these patches to http://codereview.appspot.com/, though. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Eugene Toder elto...@gmail.com added the comment: Thanks. Review link: http://codereview.appspot.com/4281051 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Nick Coghlan ncogh...@gmail.com added the comment: The review links didn't come up automatically because 336137a359ae isn't a hg.python.org/cpython revision ID. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Eugene Toder elto...@gmail.com added the comment: I see. Should I attach diffs vs. some revision from hg.python.org? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Nick Coghlan ncogh...@gmail.com added the comment: No need, since you manually created a review on appspot. The local Reitveld links are just a convenience that can avoid the need to manually create a review instance. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
New submission from Eugene Toder elto...@gmail.com: As pointed out by Raymond, constant folding should be done on AST rather than on generated bytecode. Here's a patch to do that. It's rather long, so overview first. The patch replaces existing peephole pass with a folding pass on AST and a few changes in compiler. Feature-wise it should be on par with old peepholer, applying optimizations more consistently and thoroughly, but maybe missing some others. It passes all peepholer tests (though they seem not very extensive) and 'make test', but more testing is, of course, needed. I've split it in 5 pieces for easier reviewing, but these are not 5 separate patches, they should all be applied together. I can upload it somewhere for review or split it in other ways, let me know. Also, patches are versus 1e00b161f5f5, I will redo against head. TOC: 1. Changes to AST 2. Folding pass 3. Changes to compiler 4. Generated files (since they're checked in) 5. Tests In detail: 1. I needed to make some changes to AST to enable constant folding. These are a) Merge Num, Str, Bytes and Ellipsis constructors into a single Lit (literal) that can hold anything. For one thing, this removes a good deal of copy-paste in the code, since these are always treated the same. (There were a couple of places where Bytes ctor was missing for no apparent reason, I think it was forgotten.) Otherwise, I would have to introduce at least 4 more node types: None, Bool, TupleConst, SetConst. This seemed excessive. b) Docstring is now an attribute of Module, FunctionDef and ClassDef, rather than a first statement. Docstring is a special syntactic construction, it's not an executable code, so it makes sense to separate it. Otherwise, optimizer would have to take extra care not to introduce, change or remove docstring. For example: def foo(): doc + string Without optimizations foo doesn't have a docstring. After folding, however, the first statement in foo is a string literal. This means that docstring depends on the level of optimizations. Making it an attribute avoids the problem. c) 'None', 'True' and 'False' are parsed as literals instead of names, even without optimizations. Since they are not redefineable, I think it makes most sense to treat them as literals. This isn't strictly needed for folding, and I haven't removed all the artefacts, in case this turns out controversial. 2. Constant folding (and a couple of other tweaks) is performed by a visitor. The visitor is auto-generated from ASDL and a C template. C template (Python/ast_opt.ct) provides code for optimizations and rules on how to call it. Parser/asdl_ct.py takes this and ASDL and generates a visitor, that visits only nodes which have associated rules (but visits them via all paths). The code for optimizations itself is pretty straight-forward. The generator can probably be used for symtable.c too, removing ~200 tedious lines of code. 3. Changes to compiler are in 3 categories a) Updates for AST changes. b) Changes to generate better code and not need any optimizations. This includes tuple unpacking optimization and if/while conditions. c) Simple peephole pass on compiler internal structures. This is a better form for doing this, than a bytecode. The pass only deals with jumps to jumps/returns and trivial dead code. I've also made 'raise' recognized as a terminator, so that 'return None' is not inserted after it. 4, 5. No big surprises here. -- components: Interpreter Core messages: 130955 nosy: eltoder priority: normal severity: normal status: open title: Rewrite peephole to work on AST versions: Python 3.3 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Changes by Eugene Toder elto...@gmail.com: -- keywords: +patch Added file: http://bugs.python.org/file21198/0_ast.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Changes by Eugene Toder elto...@gmail.com: Added file: http://bugs.python.org/file21199/0_fold.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Changes by Eugene Toder elto...@gmail.com: Added file: http://bugs.python.org/file21200/0_compile.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Changes by Eugene Toder elto...@gmail.com: Added file: http://bugs.python.org/file21201/0_generated.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Changes by Eugene Toder elto...@gmail.com: Added file: http://bugs.python.org/file21202/0_tests.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Changes by Eugene Toder elto...@gmail.com: -- nosy: +pitrou, rhettinger ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Changes by Andreas Stührk andy-pyt...@hammerhartes.de: -- nosy: +Trundle ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Changes by Santoso Wijaya santoso.wij...@gmail.com: -- nosy: +santa4nt ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue11549] Rewrite peephole to work on AST
Changes by Antoine Pitrou pit...@free.fr: -- nosy: +benjamin.peterson ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11549 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
