dalek | arVM: 81dabfb | timotimo++ | src/jit/emit_x64.dasc: why not have inf and neginf in addition to nan? |
00:17 | |
arVM: c2b33d0 | timotimo++ | src/jit/ (2 files): jit MVM_OP_exception (gets "current" exception object) |
|||
arVM: af93de0 | timotimo++ | src/jit/ (2 files): jit sp_get_[inso] |
|||
arVM: c6b887a | timotimo++ | src/spesh/optimize.c: this is how we could optimize getex(category|message|payload) 34c7112 | timotimo++ | src/spesh/dump.c: fill registers and BBs with spaces so they align everywhere |
|||
timotimo | not mentioned: pad the RHS of op names up to ~15 characters | 00:43 | |
(longer op names do exist, but are quite rare) | |||
03:27
nwc10 joined
06:15
FROGGS joined
|
|||
nwc10 | good *, #moarvm | 07:14 | |
07:45
Ven joined
|
|||
jnthn | moarning o/ | 07:50 | |
nwc10 | gr \o an | ||
FROGGS | o\oh deer | 07:57 | |
07:57
brrt joined
|
|||
brrt | \o | 08:02 | |
jnthn | o/ brrt | 08:08 | |
And FROGGS | |||
nwc10: Gonna apply paste.scsys.co.uk/486642 | 08:13 | ||
nwc10 | cool. I didn't bake a patch because I wasn't sure what the commit message should be | ||
heck, or even if it was the right fix | |||
jnthn | grmbl, it doesn't apply | 08:14 | |
Also now I look more closely at the patch, something is odd | 08:15 | ||
Yeah :S | 08:16 | ||
Now I look at it more in context I see your hesitation. | 08:17 | ||
Assigning a union to a variable on the stack then back to another storage slot of the same union type should not have endian issues | 08:18 | ||
nwc10 | OK, I didn't notice that part | 08:19 | |
jnthn | Well, taken together the lines are: | ||
MVMSpeshOperand category = ins->operands[1]; | |||
ins->operands[2] = category; | |||
nwc10 | it was mostly "I hit it with hammers and it seemed to work", and "fixes symptoms" without understanding why is not a valid reason to say "it's correct" | 08:20 | |
jnthn | Well, just from a C language perspective, I think this can't be the right fix. | ||
I can't believe a C compiler would mis-compile assignment of a union type... | 08:21 | ||
nwc10 | OK. I can't help you with *thinking* here, as I'm deep in some work code | ||
but I can probably apply better fixes and report back on tests, pretty fast | |||
jnthn | I fear it's going to take a bit of hunting :( | 08:23 | |
jnthn will put it aside for the moment, to work on the nasty exception/inlining bug he was planning to tackle... | 08:24 | ||
nwc10 | yes, good plan | ||
brrt | oh, waitaminute about that patch | 08:26 | |
bindexcategory tries to make a sp_bind_i, and category is uint32 | 08:27 | ||
sp_bind_i writes a int64 | |||
ergo, that spesh overwrites the return_after_unwind flag | |||
jnthn | brrt: oooooh | 08:28 | |
brrt | we should probably not just spesh struct accesses to sp_(bind|get)_[inso] | ||
jnthn | No | ||
brrt | unless we also deal with the struct sizes | ||
eh | |||
jnthn | Correct | ||
brrt | field sizes | ||
jnthn | In fact we should probably leave the op as is for interpretation, and JIT it to a field access | 08:29 | |
brrt nods | |||
anyway, we didn't implement them until justnow. maybe we should just remove them at all | 08:30 | ||
JIT can deal with them in another way | |||
add a access node | |||
or something like that | |||
(that's a plan anyway :-)) | |||
for 64 bit things, payload and message can be taken safely | 08:31 | ||
for 32 bit platforms, that won't even work | |||
jnthn | But those are object/string typed, and sp_bind_o writes a pointer size, so those are ok? | 08:32 | |
08:32
zakharyas joined
|
|||
brrt | hmm... letsee, yes you're correct | 08:33 | |
they're ok | |||
ok, with that said, we only need to add an integer field size argument to sp_get_i and sp_bind_i. and that'd fix it? | 08:39 | ||
jnthn | Well, I'd add sp_get_i32 and sp_bind_i32 | 08:41 | |
The exception/inlining bug is...tricky. Basically, we only consider one address when looking for a handler. | 08:43 | ||
Meaning if we're in inlined code we don't search the outer frame. | |||
Er, the inlining frame. | |||
Trying to walk the inline chain is going to be (a) a bit fragile since it's deopt without the deopt, and (b) need a solution for the JIT too | 08:44 | ||
Seems that we could just poke extra entries into the handlers table, however | |||
Meaning it's still a simple linear scan | 08:45 | ||
brrt | wait, i'm not sure i comprehend fully | ||
but | |||
i like that suggestion by the simple heuristic of 'make data work not algorithms' | 08:46 | ||
jnthn | *nod* | ||
brrt | and as long as you correctly add spesh annotations the JIT *should* pick them up too | ||
but its been tricky testing them | |||
jnthn | I don't think I'd add any new annotations even | ||
oh, maybe... | 08:47 | ||
jnthn looks into what would be the simplest way to do it | |||
brrt | well, i kind of need annotations in correct places in order to set the jit label markers | 08:48 | |
jnthn | *nod* | 08:49 | |
brrt | did you see the indirect-jump-to-reentry-label-hack by the way? i'm awefully proud of it's hackiness :-) | 08:50 | |
jnthn | ex->body.jit_resume_label = tc->cur_frame->jit_entry_label; ? :) | 08:54 | |
brrt | yes, that one | 09:11 | |
also the | |||
jmp FRAME->jit_entry_label | |||
which is initialized to just-after if nothing was thrown, and to the correct resumption point otherwise | |||
10:27
Ven joined
|
|||
jnthn | Well, my patch fixes the bug and survives make and make test of NQP and Rakudo | 10:34 | |
jnthn spectests it | |||
Darn, close. One test file shows up problems. | 10:52 | ||
masak | that's the most interesting kind of situation :) | 10:54 | |
"the almost-fix" | |||
jnthn | Well, got one theory about what it may be. | ||
nope :( | 10:55 | ||
Wow, with the JIT disabled the same test file does a SEGV rather than a fail | 11:00 | ||
11:13
Ven joined
|
|||
dalek | arVM/inlining-exception-fix: 7c8efa8 | jnthn++ | src/spesh/inline.c: Factor out resizing of handlers table. |
11:19 | |
arVM/inlining-exception-fix: e648c9d | jnthn++ | src/spesh/ (2 files): Account for inliner's handlers when inlining. We do this by adding extra entries to the hnadlers table that cover an entire inlinee and go to the handler in the inliner. This means we can still find applicable handlers through a linear scan, so the handler lookup code needs no changes and need not become more complex. |
|||
jnthn | nwc10: If you have time to run ^ under ASAN, especially t/spec/S32-exceptions/misc.rakudo.moar, that's be interesting. | 11:20 | |
lunch & | 11:28 | ||
nwc10 | jnthn: in setting | 12:02 | |
# Looks like you failed 6 tests of 304 | 12:23 | ||
no ASAN barfage | 12:24 | ||
paste.scsys.co.uk/487298 | 12:27 | ||
MoarVM on origin/master passes all | 12:29 | ||
(well, TODOs are TODO) | |||
everything else was fine with ASAN | |||
12:46
zakharyas joined
13:33
Ven joined
13:40
TimToady joined
|
|||
jnthn | nwc10: OK, thanks for trying | 14:16 | |
jnthn tries to figure out what on earth is going on with those six tests | 14:20 | ||
nwc10 | afk for 2 or 3 hours | 14:21 | |
& | |||
14:45
Ven joined
|
|||
jnthn | yowser, it actually explodes inside of Perl6::Optimizer while doing the EVAL of the code its testing to see if it throws an exception. | 14:45 | |
FROGGS | "yay" | 14:48 | |
15:14
lizmat joined
15:45
Ven joined
16:10
Ven joined
|
|||
jnthn | Darn, this is a really tricky bug to find :/ | 16:45 | |
16:59
Ven joined
|
|||
nwc10 | step away from the computer for a bit? | 16:59 | |
jnthn | Yeah, was just thinking about going shopping for some dinner... :) | 17:03 | |
Just discovered that the bug isn't at all the one I thought it was | |||
jnthn taeks a break | 17:13 | ||
timotimo | :o | 17:23 | |
i don't really know what bug you're hunting right now | |||
but i'm still in awe | 17:24 | ||
also ... ouch for the exception methods | 17:40 | ||
i wonder if we could do much better at optimizing inlined things like our auto-generated accessor methods | 17:41 | ||
17:41
Ven joined
|
|||
timotimo | since the name of the attribute it accesses is closed over, and when we inline we inline a particular version of the closure (right?) we could have the values as a "known value" fact ... perhaps? | 17:42 | |
i notice i really don't know much about how our closures work | |||
17:46
FROGGS joined
18:41
brrt joined
|
|||
brrt | jnthn: i probably don't have to ask if you've tested both with and without JIT | 18:42 | |
jnthn | brrt: Yeah, it's independent of that | 18:52 | |
18:54
vendethiel joined
|
|||
brrt | hmm. not sure if i'm really happy about that | 18:54 | |
seems like a difficult bug :-( | |||
fwiw, os x gave me a clean spectest this morning | 18:55 | ||
jnthn | Especially since it cropped up as a result of fixing another tricky bug. | ||
brrt | hmmpf :-) vm's are hard | 18:56 | |
timotimo | VMS is also hard, i hear | 19:07 | |
i should build a commit that turns off speshing for the getex* and bindex* ops | 19:08 | ||
or completely throws out | |||
right? | |||
brrt | probably, yes | 19:09 | |
well, actually | |||
n, s, and o are ok | |||
you can make a sp_bind_i(8|16|32|64) | 19:10 | ||
or if you like | |||
sp_(bind|get)_[bwdq] ;-) | 19:11 | ||
aaactually, if you'd go that route, why not sp_(bind|get)_[bdwqpn] | 19:13 | ||
byte word doubleword quadword pointer number | |||
that'd cover all our needs, i'd think | 19:14 | ||
you don't have to do that though, because i'll be replicating the work soon enough | |||
timotimo | :S | 19:18 | |
fair enough | |||
brrt | well.. you can if you wish :-) there might well be cases in which it's worth it (repr accesses maybe) | 19:19 | |
19:44
lizmat joined
|
|||
jnthn | cool, a thunderstorm :) | 19:52 | |
nwc10 | roof testing? | 19:53 | |
jnthn | Well, there's two stories worth of apartments between me and the roof, so if I find out about a test fail, it's a pretty epic one :) | ||
nwc10 | internal rain more likely (ie failed plumbing above) | 19:54 | |
jnthn | Aye. Fingers crossed not though...this building seems well kept. :) | ||
20:37
zakharyas joined
21:02
brrt joined
|
|||
timotimo | or flood via staircase | 21:03 | |
i'm a bit late to the topic, it seems | |||
21:34
lizmat joined
|