Ticket #77 (closed defect: fixed)

Opened 6 years ago

Last modified 12 days ago

temp allocation should be done in code generation phase

Reported by: robertwb Owned by: somebody
Priority: major Milestone: wishlist
Component: Code Generation Keywords:
Cc:

Description

This is a follow up to #67.

Change History

Changed 6 years ago by dagss

Started this here:

 http://hg.cython.org/cython-dagss/rev/33d322bd7b32

If this works, it should allow for painless, incremental migration of the temps.

Changed 6 years ago by dagss

It has a problem related to calling target_code in allocate_target_temps. Don't have more time today so need to stop here.

This only has an effect in the following testcase, which is not commited yet (so tests passes but things are not OK):

"""
>>> f()
a
"""

class A:
    attr = 0
    def __add__(self, other):
        return self
    def __repr__(self):
        return self.attr

def f():
    a = A()
    (a + 32).attr = "a"
    return a

Changed 6 years ago by dagss

Buffer access towards complex types (treating them as a struct of two floats) are now up in -dagss.

In order to get there I had to do a couple of dangerous things regarding result_code though. These patches are really small but someone with more knowledge may have more or less confidence in them than I:

 http://hg.cython.org/cython-dagss/rev/b2bcc32021fd  http://hg.cython.org/cython-dagss/rev/a8a9e4c7d5bd

Note that this should fix the CloneNode? issue I talked about. (And all tests pass btw; yet this is the kind of thing that would cause horrendous failures in obscure cases which are likely not tested for...)

Also in -dagss I've introduces NewTempExprNode?, which is a transitionary class where the temp allocation is moved to code generation. Ideally all ExprNodes? should descend from this one instead (and then it should be folded back into ExprNode? to complete the migration).

Some classes fail hard when they are changed to the new scheme. It works for BinOpNode? which now uses it. This should allow partial migration. (One must make sure a class can work with the new temp allocation scheme before constructing them in a transform after analysis).

Changed 6 years ago by dagss

There's been a long discussion about this now. See #124, some other tickets, and also  http://search.gmane.org/search.php?group=gmane.comp.python.cython.devel&query=temp+allocation+flow

Changed 6 years ago by dagss

#149 must be fixed before more nodes are converted to make the matter worse.

Changed 12 days ago by scoder

  • status changed from new to closed
  • resolution set to fixed

This is long fixed, marking as done.

Note: See TracTickets for help on using tickets.