jnorwoo / shedskin

Automatically exported from code.google.com/p/shedskin
0 stars 0 forks source link

enumerated list type inference doesn't work #99

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
class A(object):
        status_positions = ["C", "Z", "I", "D", "B", "5", "V", "N"]

        def pop_status(self):
                flags_bin = 42
                flags = set([(flag_name if (flags_bin & (1 << flag_bit)) != 0 else None) for flag_bit, flag_name in enumerate(self.__class__.status_positions)])
                print(flags)                                                                                                               

a = A()
a.pop_status()

*** SHED SKIN Python-to-C++ Compiler 0.5 ***
Copyright 2005-2010 Mark Dufour; License GNU GPL version 3 (See LICENSE)

[iterative type analysis..]
**
iterations: 2 templates: 75
[generating c++ code..]
*WARNING* a.py:8: variable 'flag_bit' has no type
*WARNING* a.py:8: variable 'flag_name' has no type

Original issue reported on code.google.com by danny.m...@gmail.com on 15 Oct 2010 at 9:34

Attachments:

GoogleCodeExporter commented 8 years ago
(these->several similar functions)

Original comment by mark.duf...@gmail.com on 4 Jun 2011 at 2:49

GoogleCodeExporter commented 8 years ago
interestingly, it's also quite simple to get a not-too-ugly backtrace (#include 
<exception>, <execinfo.h>):

 void __throw_index_out_of_range() {
-   throw new IndexError(new str("index out of range"));
+    void * array[25];
+    int nSize = backtrace(array, 25);
+    char ** symbols = backtrace_symbols(array, nSize);
+
+    for (int i = 0; i < nSize; i++)
+        printf("%s\n", symbols[i]);
+
+    throw new IndexError(new str("index out of range"));
 }

so I guess we could theoretically adorn exception objects with a more useful 
stack trace. though this may be a bit slow of course in general. hopefully we 
can also inspect the stack later on.. let's see..

Original comment by mark.duf...@gmail.com on 4 Jun 2011 at 3:11

GoogleCodeExporter commented 8 years ago
I just added this to the BaseException constructor, behind a new shedskin -x 
option. so you now get a backtrace for every exception that is created. it's 
awfully crude and slow, but at least you can see where the last exception 
originated without having to resort to gdb and diving into the shedskin code.. 
:-) seems like a useful step in the right direction.

Original comment by mark.duf...@gmail.com on 4 Jun 2011 at 4:00

GoogleCodeExporter commented 8 years ago
used this one now, to demangle the C++ symbols:

https://idlebox.net/2008/0901-stacktrace-demangled/

licensed under the WTFPL!

Original comment by mark.duf...@gmail.com on 4 Jun 2011 at 4:29

GoogleCodeExporter commented 8 years ago
Nice :-)

Hehe, WTFPL was probably created by someone annoyed by all the legalese even in 
the other Free Software licenses, huh? :-)

As for getting the backtrace being slow, well, IndexError shouldn't happen in 
normal operation anyway, so shouldn't matter too much (?)

Nice that there is a function for getting the backtrace in the first place... 
I'm aware it's not that much use when the stack frames are all optimized away - 
but it's still helpful. Even just the file&line in which the exception was 
thrown would be enough in most cases I think.

But StopException and EOFError and all these 
I-am-not-really-an-error-condition-exceptions, it would probably matter there...

As for the override being needed, well, I'm on a single-core 1.7GHz 64 bit 
(netbook) CPU here and I get about 1 fps without the override, and 20 fps with 
it.

But speed improvement maybe also can be had by profiling and finding out where 
the heck else I'm spending so much time :-) btw, does the generated Makefile 
contain a profile target?

I have a nice bug now in running international karate, it works (with a little 
less buggy graphics, yes yes working on it :-) ) only until the first punch, 
then the emulator is KO :-)

Original comment by danny.m...@gmail.com on 4 Jun 2011 at 6:06

GoogleCodeExporter commented 8 years ago
To clarify, I mean it might be enough to just have something like 
IndexError(__FILE__, __LINE__) or so and the exception remember it...

Original comment by danny.m...@gmail.com on 4 Jun 2011 at 6:13

GoogleCodeExporter commented 8 years ago
mehhh:

  File "/home/srepmub/C64/c64/cpu.py", line 1754, in BRK
    tape.call_hook(self, self.MMU, old_PC - 2)
  File "/home/srepmub/C64/c64/tape.py", line 127, in call_hook
    return(find_header(CPU, memory))
  File "/home/srepmub/C64/c64/tape.py", line 72, in find_header
    setup_tape_header(type_, header.file_name, header.start_addr, header.end_addr, memory)
  File "/home/srepmub/C64/c64/tape.py", line 50, in setup_tape_header
    memory.write_memory(buffer_addr + OFFSET_STOP_ADDR, stop_addr, 2)
  File "/home/srepmub/C64/c64/mmu.py", line 103, in write_memory
    assert isinstance(value, int), "MMU.write_memory: value is an integer"
AssertionError: MMU.write_memory: value is an integer

Original comment by mark.duf...@gmail.com on 4 Jun 2011 at 6:20

GoogleCodeExporter commented 8 years ago
btw, I've also tried setting alpha transparency of all pixels to half (since I 
swapped endianness now, the alpha transparency is at the top 8 bits) in order 
for the pixel values to always be positive, but seems ubuntu does some funky 
crt afterglow effect, then... let's see if I come to like it and commit it :-)

Original comment by danny.m...@gmail.com on 4 Jun 2011 at 6:36

GoogleCodeExporter commented 8 years ago
Usually works here (didn't change tape loading in ages I think)? what tape was 
that? :-)
try printing stop_addr out before line 50 in setup_tape_header...

Original comment by danny.m...@gmail.com on 4 Jun 2011 at 6:38

GoogleCodeExporter commented 8 years ago
Oh yeah, the AssertionErrors look kinda strange because an assertion is 
something that is to be true, so the AssertionError looks kinda strange: "well 
duh, it's supposed to be an integer" :-)

Original comment by danny.m...@gmail.com on 4 Jun 2011 at 6:40

GoogleCodeExporter commented 8 years ago
Oh yeah, the loaders/t64.py tape loaders fixes up the end_addrs since many 
images have it wrong, maybe there's a bug there...

Original comment by danny.m...@gmail.com on 4 Jun 2011 at 6:42

GoogleCodeExporter commented 8 years ago
Oh yeah, the backtrace looks nice :-)

Original comment by danny.m...@gmail.com on 4 Jun 2011 at 6:44

GoogleCodeExporter commented 8 years ago
that's for intkarat.t64. stop_addr is 41034. 

about __FILE__ and __LINE__: this wouldn't help I think, because it would show 
something like: lib/builtin.hpp:1234:__throw_index_exception(), hardly more 
useful than just 'IndexError, somewhere'.

about the negative integers.. I haven't looked at the code, but if you only use 
bitwise operators, I would think it doesn't matter if the shedskin 'int' is 
signed or not..?

Original comment by mark.duf...@gmail.com on 4 Jun 2011 at 7:10

GoogleCodeExporter commented 8 years ago
as for the override, have you tried recently with shedskin GIT and the 
array('I') solution..? that should be so fast now, that the override should not 
be necessary. I measured it could do 1000 screens or so per second here..

Original comment by mark.duf...@gmail.com on 4 Jun 2011 at 7:13

GoogleCodeExporter commented 8 years ago
forgot one thing, shedskin -x also adds -fno-inline now, so you should get to 
see everything, of course at some cost.

Original comment by mark.duf...@gmail.com on 4 Jun 2011 at 7:45

GoogleCodeExporter commented 8 years ago
Tried right now and you are right, it's very fast now O_o 
The override is only twice as fast as the normal thing now and both are usable 
enough I think.

Ah, you could use the #line preprocessor directive to line up the line numbers 
with how they are in the py file... then they would match up... Well, I agree 
it's getting rather convoluted by then... (see 
http://gcc.gnu.org/onlinedocs/cpp/Line-Control.html )

Yes, the renderer is only using bitwise operators, however the converter which 
converts the shedskin int array to a CPython int array did not like them... I 
got an error on the array() constructor I think...

Original comment by danny.m...@gmail.com on 4 Jun 2011 at 8:40

GoogleCodeExporter commented 8 years ago
or I had that array() call in the c64_main part, uselessly? Not so sure 
anymore...

Original comment by danny.m...@gmail.com on 4 Jun 2011 at 8:42

GoogleCodeExporter commented 8 years ago
as long as it works well for you without the override, I'm happy.. :-)

uuuhm, yeah, I guess on conversion we have to make the integers negative again, 
so that should explain the problem. after removing this it still works fine, 
and I guess it should bring performance close to the override method:

data = [(2**32+item) if item < 0 else item for item in self.data]

yeah, we could certainly try to hide the C++ code here, but I actually like the 
transparency. I'd really like to also add cpp filenames and line numbers. and 
I'm still hoping to construct backtraces after the fact, but if that was 
remotely possible, why can't I find anyone doing this already.. 

Original comment by mark.duf...@gmail.com on 4 Jun 2011 at 9:05

GoogleCodeExporter commented 8 years ago
ah, no wait, I pressed 'send' too soon.. :-) I'm still testing without this 
line, but already had my hopeful conclusion ready.. 

Original comment by mark.duf...@gmail.com on 4 Jun 2011 at 9:06

GoogleCodeExporter commented 8 years ago
alright, yes, I can confirm that you can remove this line now.

I'm seeing 70% cpu usage now the default time setting of 20.. not sure why it's 
more now. perhaps I measured at 90 last time, dunno. 

more importantly, I now get into the game! :D the screen is garbled though, and 
it looks like it indeed dies at the first strike. 

the whole bootup process is now wonderful btw, the text in top seems to come up 
much more realistically, and responsiveness to typing is perfect.

Original comment by mark.duf...@gmail.com on 4 Jun 2011 at 10:08

GoogleCodeExporter commented 8 years ago
btw, interestingly, after typing a few random words after bootup (and getting 
some ?out of memory errors), and then running the game, it doesn't crash but 
keeps going..

Original comment by mark.duf...@gmail.com on 4 Jun 2011 at 10:11

GoogleCodeExporter commented 8 years ago
hmm, HEAD seems broken.. with this change it works again for me:

--- c64/screens.py  (revision 1485)
+++ c64/screens.py  (working copy)
@@ -27,7 +27,7 @@
        self.CIA2 = CIA2
        #self.pixbuf = (HEIGHT * WIDTH) * [0]
        self.props = VIC.props
-       self.current_scanline = (WIDTH + sprites.WIDTH * 2) * [0]
+       self.current_scanline = WIDTH * [0]

Original comment by mark.duf...@gmail.com on 5 Jun 2011 at 6:04

GoogleCodeExporter commented 8 years ago
so the mmu assert(value, int) problem was caused by stream.tell() returning 
longs on my system. using int(stream.tell()) in prg.py and t64.py fixes that.. 
so I can load intkarat.t64 again, but it crashes once it starts to draw 
sprites.. :P

Original comment by mark.duf...@gmail.com on 5 Jun 2011 at 6:43

GoogleCodeExporter commented 8 years ago
expand_vertically was used before it was assigned. this seems to fix that, but 
now the game screen is more garbled than before. it doesn't seem to crash 
anymore though :)

+           expand_vertically = (2 if props.sprite_expand_vertically & sprite_selector 
else 1)
+           expand_horizontally = (2 if props.sprite_expand_horizontally & 
sprite_selector else 1)
            if client_index >= sprite_Y and client_index < sprite_Y + sprite_size_Y and ((props.sprite_priority & sprite_selector) == 0) == B_foreground and props.sprite_enabled & sprite_selector:
                client_Y = (client_index - sprite_Y) >> (expand_vertically - 1)
                cell_inner_Y = client_Y & 7
                for k in range(sprites.WIDTH):
                    sprite_scanline[k] = 0
-               expand_vertically = (2 if props.sprite_expand_vertically & sprite_selector 
else 1)
-               expand_horizontally = (2 if props.sprite_expand_horizontally & 
sprite_selector else 1)

Original comment by mark.duf...@gmail.com on 5 Jun 2011 at 7:00

GoogleCodeExporter commented 8 years ago
tried a few .t64 games off the web, and found it loads no entries at all, 
because cur_files is 0. using max_files instead, I was able to get two games 
going a bit:

-lode runner (_loda.t64) says 'decompacting' and has the flashy borders. then 
it crashes with an 'ISC not implemented'.
-tetris.t64 says 'hotline' in the top of the screen, and seems to not do 
anything after that..

Original comment by mark.duf...@gmail.com on 5 Jun 2011 at 7:52

GoogleCodeExporter commented 8 years ago
-pitfall (pitfallfile.t64) shows a black 'decrunching' screen, and then 
something that looks like a garbled text.

Original comment by mark.duf...@gmail.com on 5 Jun 2011 at 9:33

GoogleCodeExporter commented 8 years ago
--- c64/screens.py  (revision 1485)
+++ c64/screens.py  (working copy)
@@ -27,7 +27,7 @@
        self.CIA2 = CIA2
        #self.pixbuf = (HEIGHT * WIDTH) * [0]
        self.props = VIC.props
-       self.current_scanline = (WIDTH + sprites.WIDTH * 2) * [0]
+       self.current_scanline = WIDTH * [0]

That is so that a sprite can stick out of the right side of the screen (i.e. 
partially outside)... I've changed pixbufs.py to just always copy only WIDTH 
items now, so better not change current_scanline length to "too short".

I've committed your stream.tell() fix now.

And the expand_vertically fix too... how embarrassing, I didn't find that bug 
for ages (it worked "most" of the time here, so hard to get a hold on) :-)

Yeah, there are tons of broken T64 files on the web :-( I've thought about just 
hardcoding 1 entry at minimum... not sure whether it would fall into the 
"workaround I'm going to regret" category, though...

Still no game working enough... freaky problems...

If you want to debug it some more, you can try the "toggle disassembly" button 
(or set B_disasm of the CPU instance to True) to find the surroundings 
(redirect into file or it's going to drive you crazy)... it's actually nice if 
it breaks on an obscure instruction - just check the second-to-last instruction 
usually.

As for the unimplemented instructions, these are technically valid instructions 
but "obscure" ones. The reason they are not implemented is so I can find out 
easily when the guest CPU wanders off into data or gets out of step (i.e. in 
order to increase the probability of junk not being a valid opcode).
ISC is increment and then subtract something again... would be no problem 
implementing it, if it was used for real... unlikely it was used for real, 
though :-)

Original comment by danny.m...@gmail.com on 6 Jun 2011 at 7:57

GoogleCodeExporter commented 8 years ago
to clarify, these obscure instructions were undocumented back in the day, so 
the only way to find out about them would have been trying to brute-force all 
possible numbers as opcodes... that's why it's unlikely they have been used 
(well, maybe by demo people...)

Original comment by danny.m...@gmail.com on 6 Jun 2011 at 8:06

GoogleCodeExporter commented 8 years ago
you'll get there.. :-) 

maybe it's an idea to debug using smaller programs (such as demos?) first, and 
slowly build up to larger games? after some googling I quickly found a few 
small programs of less than 4 kb, and several of these trigger illegal 
instructions as well, or don't seem to 'show' much on the screen.. should be 
more fun to debug than something like intkarat or giana.. ;-)

or perhaps you could write something yourself, testing various areas, to see if 
the emulator can handle that already..? there are C compilers available. also 
sounds like more fun than staring at infinite code streams.. :-)

I'm just making some wild suggestions here, no idea how you are debugging this 
now..

unfortunately no time to play further at the moment :-///

Original comment by mark.duf...@gmail.com on 6 Jun 2011 at 9:03

GoogleCodeExporter commented 8 years ago
hmm, lots of nice demos here apparently, but unfortunately all seem to use 
.d64..

http://noname.c64.org/csdb/

Original comment by mark.duf...@gmail.com on 7 Jun 2011 at 3:00

GoogleCodeExporter commented 8 years ago
woah, apparently there's even a 256 byte competition, didn't know that :-)

http://noname.c64.org/csdb/release/?id=99758

Original comment by mark.duf...@gmail.com on 7 Jun 2011 at 3:02

GoogleCodeExporter commented 8 years ago
sorry for all the spam, but did you see the screenshots I sent you..? ;-)

Original comment by mark.duf...@gmail.com on 9 Jun 2011 at 7:55

GoogleCodeExporter commented 8 years ago
About the 256 byte competition, interesting... Plus, made me remember I still 
haven't done SID support, but I should... *gets headphones*

The http://noname.c64.org/csdb/release/?id=99758 doesn't even load yet... too 
bad :-)

(Yes, did see the screenshots, very nice :-) but you know that ^^)

Original comment by danny.m...@gmail.com on 25 Jun 2011 at 5:54

GoogleCodeExporter commented 8 years ago
no more tabs, woohoo, thanks! :D

I looked into the slow pixbuf scaling a bit, and apparently INTERP_TILES is not 
the fastest.. INTERP_NEAREST is faster according to the documentation. 
compiling now to see if this makes a big difference.. but perhaps this scaling 
method is just not meant for 50 FPS.. :-) maybe I will try to build up a 
doubled screen in the extmod to see what happens..

Original comment by mark.duf...@gmail.com on 25 Jun 2011 at 7:27

GoogleCodeExporter commented 8 years ago
yup, I can confirm this brings the framerate here from about 30 to a constant 
50 FPS.

Original comment by mark.duf...@gmail.com on 25 Jun 2011 at 8:01

GoogleCodeExporter commented 8 years ago
Daamn... trying that here now... (also updated shedskin and recompiling c64 
using it, so will take some time... let's see)

Original comment by danny.m...@gmail.com on 25 Jun 2011 at 8:04

GoogleCodeExporter commented 8 years ago
Yeah, much faster here too...

How did you do the FPS measurement?

Original comment by danny.m...@gmail.com on 25 Jun 2011 at 8:54

GoogleCodeExporter commented 8 years ago
     def fire_timer(self):
+        if self.screen_count % 50 == 0:
+            print 'FPS: %.2f' % (50*(1/(time.time()-self.t0)))
+            self.t0 = time.time()
         self.C64.fire_timer()
+        self.screen_count += 1
         return True

Original comment by mark.duf...@gmail.com on 25 Jun 2011 at 9:01

GoogleCodeExporter commented 8 years ago
Thanks :-)
Idling on the start screen (without override) gives me about 30 fps.
With override I get 40 fps.
Nice :-)

Original comment by danny.m...@gmail.com on 26 Jun 2011 at 5:33

GoogleCodeExporter commented 8 years ago
must.. kill.. override.. :-))

Original comment by mark.duf...@gmail.com on 26 Jun 2011 at 8:23

GoogleCodeExporter commented 8 years ago
it looks like you don't depend on IndexError, so perhaps shedskin -b can also 
help. I haven't tried, because I'm trying to make a profile first. it would 
really help here as well I guess if we could save/load the c64 state, so we can 
easily profile at any point in time in a game. then we can just compile c64.py 
as a non-extmod, make it load the state, run a while, and feed the results to a 
profiler.

Original comment by mark.duf...@gmail.com on 26 Jun 2011 at 10:13

GoogleCodeExporter commented 8 years ago
hum, -b makes the raw emulator during the start screen about twice as fast 
here.. :-) (shedskin c64 -ob && make c64_prof etc..). see attachment for the 
profile using shedskin -ob.

Original comment by mark.duf...@gmail.com on 26 Jun 2011 at 10:58

Attachments:

GoogleCodeExporter commented 8 years ago
Yeah, shedskin -ob c64 without (!!) override leads to 47 FPS now here.
Adding dump and restore now, let's see...

Original comment by danny.m...@gmail.com on 26 Jun 2011 at 12:58

GoogleCodeExporter commented 8 years ago
commit save/restore functionality which seems to work

Original comment by danny.m...@gmail.com on 26 Jun 2011 at 3:09

GoogleCodeExporter commented 8 years ago
:D nice! seems to _almost_ work for intkarat here. see attachment for dumped 
state..

Original comment by mark.duf...@gmail.com on 26 Jun 2011 at 3:18

Attachments:

GoogleCodeExporter commented 8 years ago
Yeah, forgot the VIC registers... try now...

Original comment by danny.m...@gmail.com on 26 Jun 2011 at 5:39

GoogleCodeExporter commented 8 years ago
works perfectly.. :D 

I had a look at the intkarat sprite problem with this, but I can't find the 
root cause.. between scanline 188 and 189, the program probably wants to switch 
sprite_y for all the sprites to 189, but instead it only switches half, and the 
other half one line later. so half of the sprites are not drawn correctly on 
this line.. 

VIC-II $1 := 189
VIC-II $3 := 189
VIC-II $5 := 189
sprite: 5 client_index: 189 sprite_y: 168 <- yegh!
sprite: 4 client_index: 189 sprite_y: 168 <- yegh!
sprite: 3 client_index: 189 sprite_y: 168 <- yegh!
sprite: 2 client_index: 189 sprite_y: 189
sprite: 1 client_index: 189 sprite_y: 189
sprite: 0 client_index: 189 sprite_y: 189
VIC-II $7 := 189
VIC-II $9 := 189
VIC-II $B := 189

Original comment by mark.duf...@gmail.com on 26 Jun 2011 at 6:47

GoogleCodeExporter commented 8 years ago
How does International Karate find out which scanline it is on? Usually it's 
done like this: There's a register (set by writing to vic_ii.py 
A_RASTER_COUNTER, that is, address $D012) which will set 
self.screen.breakpoint_raster_position which "screen.py" will honor and raise 
an interrupt when on that line. Maybe the screen rows are traversed faster than 
the CPU can find out that they are? Not sure...
I'm also not sure whether we are supposed to set (and notify) which row the 
current row is BEFORE we draw it or AFTERWARDS, but I thought it would make 
sense to handle it before, since then the guest program can still do something 
to the row, which it seems to be able to do for half in that case (??).

Original comment by danny.m...@gmail.com on 26 Jun 2011 at 7:29

GoogleCodeExporter commented 8 years ago
yeah, it seems to use this. it sets a breakpoint at value 186. but it looks 
like the emulator breaks _after_ drawing line 186, as it checks against 
client_raster_position, which is 1 behind raster_position. strangely, when I 
change this as follows, it doesn't change anything - the vic registers $1 
through $B are still set at the same time. but the rest of the game is half 
broken. no time unfortunately to look into this further now.. :/

-        if self.B_enable_raster_interrupt and self.client_raster_position == 
self.breakpoint_raster_position:
+        if self.B_enable_raster_interrupt and self.raster_position == 
self.breakpoint_raster_position:

Original comment by mark.duf...@gmail.com on 26 Jun 2011 at 8:38

GoogleCodeExporter commented 8 years ago
The guest can also read the A_RASTER_COUNTER, so if you change the condition 
like above then you should also change read_memory A_RASTER_COUNTER to match 
(after all, maybe the guest double-checks whether it's on the correct row in 
the interrupt handler).
That's in vic_ii.py read_memory for A_RASTER_COUNTER and the highest (9th) bit 
of the value is in A_CONTROL_1 self.control_1. 

So long story short, best just change the value of client_raster_position in 
screen.py in order to test instead. The reason that variable exists is 
specifically to lie to clients, so no worries when messing with it... :-)
Yeah, I'm also done for today and probably will fall asleep in the chair if I 
don't stop now ;-)

Original comment by danny.m...@gmail.com on 26 Jun 2011 at 9:00