00:26 Zoffix joined
Zoffix What's the difference between `if_s` and `if_s0` ops? 00:27
Also, how would I go about adding `if_u` and `unless_u` ops?
Reading the code, I surmise `if_s0` might be faster, but then is there any reason to ever use `if_s`? 00:29
hm, ok, looking at ancient commit, looks like `if_s0` versions were used to check if `"0"` was true, back when Perl 5's semantics for Str boolification were used github.com/MoarVM/MoarVM/commit/43...72879993bb 00:37
Now, I just need to add the `if_u` and `unless_u` ops. I assume `if i32` and the like aren't really needed and those cases can be handled by 64 bit versions. 00:38
timotimo Zoffix: how would if_u and unless_u even differ from if_i and unless_u? 00:44
the only false value in both cases is 0x0
Zoffix timotimo: is it the same for both signed and unsigned? I guess I didn't think about it far enough 00:46
I mean, my thought process was: "uint coerced to int could overflow to be zero" but I didn't check if that's actually true
00:47 psch joined
timotimo nah, sufficiently high numbers will appear to be negative if coerced from uint to int 00:47
but only 0 maps to 0 00:48
Zoffix Cool. That makes my job easier. timotimo++
timotimo :) 00:49
00:55 Zoffix left 00:56 AlexDaniel joined
AlexDaniel dogbert17: github.com/MoarVM/MoarVM/issues/757 00:56
01:17 unicodable6 joined, quotable6 joined 02:30 Zoffix joined
Zoffix Hm. I guess it's not as easy :) Getting "At Frame 1, Instruction 27, op 'unless_i', operand 0, MAST::Local of wrong type (3) specified; expected 4" when trying to feed `nqp::if` with int32 02:32
(also, I'm noticing the if/unless seem to be flipped)
02:57 ilbot3 joined
Zoffix samcv: are there docs for how to add ops? 02:58
03:33 squashable6 joined 03:47 Kaiepi joined 03:57 bloatable6 joined 03:58 unicodable6 joined
samcv Zoffix: uhm. not sure 04:13
you edit oplist and then you run tools/update_ops.p6 and then you have to add it to interp.c 04:14
then it's added to moarvm and you can add it to nqp then
04:14 mtj_ joined
Zoffix ok. thanks 04:15
the info at the header of oplist is confusing... it says new ops must come at the end of the file, but then says "the exception to this rule" are the spesh ops which must come "to the very end of file"... So... Both of them go to the end of file, then why is one an "exception". And do new ops get added *before* spesh ops? Since spesh can be renumbered that'd make sense to me, but AFTER spesh ops I also see 04:21
`ctw_check` and other ops, so are spesh ops aren't special?
samcv Zoffix: uh the last non spesh op 04:28
Zoffix samcv: are all spesh ops prefixed with "sp_"?
samcv yep
so above: # Spesh ops. Naming convention: st
Zoffix samcv: but there are now more non-ops after spesh ops: github.com/MoarVM/MoarVM/blob/mast...#L961-L984 04:29
samcv yeah ignore those
Zoffix ok 04:30
samcv add it below coerce_II
np :)
glad to help
Geth MoarVM: a7a26eb116 | (Zoffix Znet)++ (committed using GitHub Web editor) | src/core/oplist
Make it more clear where new ops are meant to go to

Per: irclog.perlgeek.de/moarvm/2018-01-18#i_15704579		 04:31
MoarVM: samcv++ created pull request #781:
UCD Cleanup		 05:22
samcv need to merge a few more commits in here. but taken me some time to write comments for all the 15 or so commits i locally made
hah. it doesn't render the diff for tools/ucd2c.pl since "large diffs are not rendered by default" 05:23
05:30 Zoffix left
AlexDaniel samcv: oh, hello! Are you up for the release this weekend? 05:40
samcv yeah
releasable6: status 05:41
releasable6 samcv, Next release in 2 days and ā‰ˆ13 hours. Blockers: github.com/rakudo/rakudo/issues?q=...%9A%A0%22. Unknown changelog format
AlexDaniel cool!
05:50 committable6 joined, squashable6 joined, statisfiable6 joined 06:33 domidumont joined 06:39 domidumont joined
nwc10 jnthn: occasional heap-use-after-free in t/spec/integration/eval-and-threads.t - paste.scsys.co.uk/566406 07:36
nine Oh, apparently inline_in_place does make a measurable difference in performance 07:41
07:43 zakharyas joined 07:50 zakharyas joined 09:22 domidumont joined 09:48 Kaiepi joined
jnthn nine: Hoped it would :-) 10:30
nine: Note that when we BB fuse to help the JIT, then an inline can be fused wiht the blocks surroudning it too, and the lot JITted as a single expression JIT expression 10:31
11:05 domidumont joined
nwc10 jnthn: and again paste.scsys.co.uk/566409 12:22
nine I guess I better wait with merging inline_in_place till after the release? 12:29
lizmat nine: good question 12:35
if I understand correctly, it *is* basically performance improving code
with some bugfixes on the side ?
nine lizmat: yes, it's a pure performance improvement. The bugs that were uncovered and fixed were there before but did not hurt as a side effect of how the spesh graph was structured. 12:37
lizmat and the thing nwc10 has spotted, has nothing to do with this ?
nine quite sure that's unrelated 12:38
Btw. I really like how simple memory management is in spesh code :)
jnthn nine: Yes, let's do it after the release 12:48
nine jnthn: btw. I'm not sure I've communicated my question about the end boundary check well enough yesterday. Right now we check offset <= cand->inlines[i].end. But if offset points to the instruction _after_ the one triggering the deopt and cand->inlines[i].end is the offset of the last instruction in the inline (inclusive), we are still of by one, aren't we? 12:53
jnthn fwiw, running one $dayjob tool on HEAD of everything gives the occasional SEGV as of quite recently... :( 12:54
I'll see if I can figure out what's doing it 12:55
nine: Hm, I had thought .end is exclusive
Hmm... 12:56
Oh. Now I see INLINE_END in src/spech/codegen.c is actually set to the offset *before* emitting the instruction 12:57
And I guess when we knew there was always a goto at the end that was OK as the annotation was on the goto
Perhaps we should make it emit that post
Otherwise, yes, it's off by actually the size of the last instruction 12:58
nine If we're gonna do that, we'll have to revert the fix to MVM_frame_find_contextual_by_name 13:15
Also this means that we can't do a MVM_spesh_candidate_find_inline_by_code_position anymore, since in one place we need an inclusive check and in the other we don't. 13:17
13:22 zakharyas joined
nine jnthn: it may sound correct but actually breaks :) 13:29
dogbert2 AlexDaniel: wrt libuv: github.com/MoarVM/MoarVM/pull/752 14:05
14:14 bisectable6 joined 14:31 lizmat joined 14:39 releasable6 joined, greppable6 joined 14:53 zakharyas joined 14:57 zakharyas joined 15:23 bart__ joined
brrt \o 15:42
brrt has decided to dig in and help figure out what to do with the frame handler starts and stops and stuff 15:47
15:53 ilbot3 joined 16:05 zakharyas1 joined
nine So....now that my quick introduction into spesh's inlining code is taken care of, what was that about spesh not carrying over all facts when inlining? The thing that's often preventing native calls from getting JIT compiled. 17:11
17:14 hoelzro joined 17:23 coverable6 joined, benchable6 joined
timotimo we were not optimizing the bbs that were added into the graph 17:35
or maybe we forgot to run a fact discovery pass over them?
17:49 lizmat joined 17:55 domidumont joined 18:04 zakharyas joined 18:41 reportable6 joined, nativecallable6 joined 18:53 _Kaiepi joined 19:00 Kaiepi joined 19:56 zakharyas joined, Ven` joined 20:12 Ven` joined
nine I don't get why merging those 2 basic blocks would cause any issues: gist.github.com/niner/75101ddf67bf...b25c65ec49 20:32
But somehow it does
Result looks like this: gist.github.com/niner/05d4b5cefc30...c2114def0e 20:34
jnthn unless does control flow
See graph.c's initial basic block building for the rules on what a basic block is
Those must still be upheld by the merging 20:35
nine jnthn: so basically instructions with an MVM_operand_ins? 20:40
jnthn Yes 20:42
However
It's easier than that: only blocks with one successor that is equal to the next block are eligigle for merging 20:43
Ah, and the following block should have one pred
So you don't need to do that whole load of analysis, it's just a look at the succ/pred 20:44
And then looking at the final instruction and making sure it's not invokish
I think that's probably sufficient
The succ/pred encodes flow control, including exception handler stuff 20:45
A common case where stuff can be merged will be where a decont got rewritten into a sp_p6oget_o or similar
That is, we saw it's just a simple pointer deref 20:46
nine Aren't exception handlers also possible successors of a block?
jnthn We model control exception handlers that way
We don't model catch exceptions so precisely
Those are all linked from the entry basic block at present 20:47
(Used to be that way for control ones too, but it turned out we could get better data propagation through PHI merges if things like loop flow control were better represented) 20:48
We may be able to merge more BBs if we can detect the case where an invokish ceased to exist 20:49
And so the edge from that BB to the handler can go away
We don't currently, I believe, have the data handy to know that
20:51 evalable6 joined
nine Yep, works much better already :) 20:56
Nice, big basic blocks. 21:00
jnthn
.oO( I like big blocks and I cannot lie... )
 21:02
nine A hand full of test files still fail with errors that remind me very much of what I've seen during my inline work 21:09
21:15 Ven` joined
nine And indeed, preventing inlined BBs from getting merged fixes those issues. 21:16
timotimo there ought to already be a nice win if we only go for non-inlined BBs for fusing 21:31
nine Eeeexcept for t/spec/S04-declarations/constant.rakudo.moar which fails with "Cannot bind attributes in a F::B type object" but _only_ if there's no MVM_SPESH_LIMIT 21:33
timotimo o_O
nine Even a ridiculous limit like MVM_SPESH_LIMIT=500000000 fixes is
timotimo can you try spesh blocking? 21:35
could just be timing differences
(but ... how)
nine I always run with blocking and nodelay 21:36
timotimo that's good
in the future i want to build a tool that properly does this, but if you turn all 0xabcd0123 strings into the same string you should be able to more or less diff two spesh logs. 21:38
nine I've done that in vim quite often :)
timotimo fair enough
seems like i have nothing to teach you :( 21:39
btw did you see my gdb python script that gives you a step-by-step of the optimization process?
nine nope
timotimo tools/trace_spesh_optimizer.gdb 21:40
i'm a little bit proud of it :D
probably not helpful to your current task, though
Geth MoarVM/inline_in_place: 25d1e7941f | (Stefan Seifert)++ | src/spesh/optimize.c
Merge non-control-flow basic blocks

Due to other optimizations we often end up with blocks of code that are split over multiple basic blocks for no good reason. Reassemble larger blocks out of those so the expression JIT has more code to optimize.		 21:42
MoarVM/inline_in_place: 73f34b1b2b | (Stefan Seifert)++ | src/spesh/optimize.c
Remove more pointless gotos

We will remove empty basic blocks in merge_bbs later on, so no harm in creating them.		 21:43
nine Ok, today's work is save. Time for bed :) 21:44
timotimo good night nine!
excellent work
nine Good night!
jnthn nine: Nice work! About inlined BBs getting merged: a BB graph is produced from the optimized bytecode, so (unless we did further transforms on the inlinee, which we don't do much of at the moment) then the inlinees should already be in minimal BB form 22:24
So it's worth checking if there's a condition missing in the merger
22:44 alexk6 joined
alexk6 nine: is the line github.com/MoarVM/MoarVM/commit/25...067a3R2417 in merge_bbs correct? shouldn't it be: if (!bb || !bb->linear_next) return; /* looks like there's only a single bb anyway */ 22:54
jnthn alexk6: Looks like it to me 22:56
Nice catch :)
22:57 lizmat joined

Powered by Cro and the RakuĀ® Programming Language.

Please report any issues / comments / feature requests as an issue on App::Raku::Log.

Thank you!