Geth MoarVM: 4e51615425 | (Samantha McVey)++ | 3 files
ucd2c.pl: Fix White_Space/space Unicode Property +many other improvements

  * Fix Property Name 'space' so it doesn't incorrectly refer to the
   Line_Break=space property and fix White_Space property so it doesn't refer to
   Bidi_Class=White_Space and instead refers to the White_Space property.
   * Fixes "\t" ~~ /<:space>/ which now correctly matches
   * Fixes "1".uniprop('White_Space') so it doesn't return `True`
... (14 more lines)		 00:55
MoarVM: cd902eb585 | (Samantha McVey)++ (committed using GitHub Web editor) | 3 files
Merge pull request #786 from samcv/White_Space

ucd2c.pl: Fix White_Space/space Unicode Property +many other improvements
01:12 wander joined
japhb samcv: Are you doing the MoarVM release? IF so, when it is? 01:27
*is it 01:28
Dang slow connection
samcv well after AlexDaniel runs toaster i'll release then
japhb (y)
01:33 yoleaux joined 01:37 harrow joined 02:58 ilbot3 joined 03:50 Geth joined
MasterDuke anyone have an opinion on whether i should continue creating new branches and PRs for each new JIT template i add? 03:57
unless they get merged relatively quickly it can get annoying dealing with potential merge conflicts depending on where in the file i add them 03:59
should i combine into fewer PRs with several templates per? 04:00
i'm off to sleep, but i'll backlog for any opinions or alternate suggestions 04:01
04:53 nativecallable6 joined 05:50 statisfiable6 joined 06:29 brrt joined
brrt good * #moarvm 06:35
yoleaux 23 Jan 2018 22:48Z <jnthn> brrt: Having pondered it a little, sp_findmethod is really "we couldn't do a good opt on the method call here". From the spesh logs I've looked at of late, they are all on code paths that we no data for - meaning they were cold. Given that, being clever and trying to make it a half-invokish maybe isn't worth it.
brrt hmm, good point 06:36
first frame we compile it on is already taking the slow path
samcv good * brrt 06:47
brrt good * samcv 06:48
i might know why that is though
(why it is on the slow path)
it is because i compare the address in the spesh slot with the value it is expected to be on 06:49
06:58 AlexDaniel joined
AlexDaniel just woke up :) yeah, kicked off the toaster now 06:59
brrt \o AlexDaniel 07:05
hmm, i was wondering why my idx const optiization was not applied, then i renemebered that i'm not looking at the jit-expr-optimizer output 07:11
07:33 domidumont joined 07:42 brrt joined 07:48 domidumont joined
samcv AlexDaniel: great 08:00
i'm gonna try to head to bed. will be back in the morning 08:01
AlexDaniel k cool 08:16
08:27 domidumont joined 09:06 klapperl joined 09:10 nebuchadnezzar joined 09:24 zakharyas joined
nine jnthn: what would be a reason for employing sp_findmethod when there's already a speshed version of the invokee? 09:45
09:54 zakharyas1 joined 09:56 zakharyas joined
jnthn nine: When we can't, at spesh time, resolve what the invokee is 10:06
It's just a variant of findmeth that does a monomorphic cache, which might or might not help matters. 10:08
(But in the case it doesn't, shouldn't hurt)
nine jnthn: that's surprising though. There is only one candidate for p5_sv_to_char_star and that is the one in Inline::Perl5::Interpreter. Why would we not be sure of that? 10:13
jnthn No idea. Is it a normal method (no FALLBACK shenanigans)? Does spesh know the type of the invocant? 10:15
nine MasterDuke: the release will hopefully be done soon (tm) and then we can merge all those PRs and the problem should go away.
jnthn (The facts section of the log can tell you the latter)
nine Facts are: r18(3): usages=12, flags=4 Dcntd 10:17
No FALLBACK or anything. Inline::Perl5::Interpreter is really just a container for those low level methods. 10:18
r18(3) comes from: sp_p6oget_o r18(3), r17(3), liti16(16) 10:19
jnthn Dcntd means it only knows it isn't containerized
nine With facts on r17(3) being r17(3): usages=14, flags=265 KnTyp Concr LogGd
jnthn Not that it knows a type
Hmm 10:20
That is usually from a rewritten getattr
nine The line in the code is: $!p5.p5_sv_to_char_star(value)
With $!p5 being the Inline::Perl5::Interpreter contained in the Inline::Perl5 object. Set once and never changes. 10:21
jnthn So, next step is to look back to the spesh stats gathered for this routine
And see what data was collected from the log of the getattr instruction
*logging
nine Would that be this? "Type 0: RW Scalar (Conc) of Inline::Perl5::Interpreter (Conc)" 10:24
That's from the type tuple for "252 x static frame 'p5_sv_to_char_star' (301) (caller is outer: 0, multi 0)" in the statistics for statistics for 'p5_to_p6' 10:25
jnthn I don't think so, it will be in the...uh...forgot what the section is called, but offset log or something 10:26
Probably a bit beneath that
It'll have an integer bytecode offset and then something on types logged at it
nine That is from the "Logged at offset:ā¤¶664:ā¤¶252 x type Str (Conc)ā¤¶252 x static frame 'p5_sv_to_char_star' (301) (caller is outer: 0, multi 0)ā¤¶252 x type tuple:ā¤¶... 10:30
Other than that there's just "Static values:ā¤¶- Sub+{is-pure}+{Precedence} (0x217a4c0) @ 284ā¤¶...
Probably much easier to read: gist.github.com/anonymous/1480534d...1-txt-L124 10:35
jnthn Ah, but this is 10:36
Latest statistics for 'p5_to_p6'
nine Which is the caller of p5_sv_to_char_star
jnthn We'd need Latest statistics for 'p5_sv_to_char_star'
Yeah, but the missing opt is in p5_sv_to_char_star I thought?
Or is it in this one?
10:36 TimToady joined
nine That one's even more straight forward: gist.github.com/anonymous/d9b208c9...f195059903 10:37
The missing opt is in p5_to_p6 which uses sp_findmeth to find p5_sv_to_char_star
jnthn Aha
So, question is which of these offsts is the decont in question
If you look back to the before optimization code, probably there are some log annotations in there. Find the one on the decont that got optimized, and there should be an offset that matches an entry under Logged at offset 10:38
nine Yeah the getattr_o is Logged (bytecode offset 610) 10:39
298 x type Scalar (Conc)
jnthn OK, and is there a logged decont beneath it? 10:41
Presumably that's
624:
298 x type NativeCall::Types::Pointer (Conc)
nine No, the decont is not logged 10:42
jnthn oh, but that makes no sense
Hmmm
10:42 benchable6 joined, bisectable6 joined
jnthn But why... 10:42
ah, short meeting, bbi15
10:42 coverable6 joined 10:45 releasable6 joined
jnthn back 11:01
11:04 psch joined
nine Where do the log annotations come from usually? 11:25
jnthn In the case of decont, github.com/MoarVM/MoarVM/blob/mast...rp.c#L2991 11:29
nine I see 5 conditions that have to be met for the decont to actually get logged. So at least one of them isn't met. 11:41
jnthn: the if (prev_op - 4 == *(tc->interp_cur_op)) condition in MVM_spesh_log_decont never succeeds. Ever. 12:06
jnthn: and I don't see how it could. Shouldnt that be prev_op + 4?
But I don't know what the purpose of the check is anyway 12:07
The way I read it, it's: prev_op = cur_op, cur_op += 4, ->fetch, "are we still where we were before the fetch, i.e. did the fetch invoke something?" 12:15
In that case, we do have to compare prev_op + 4 with our current position, not prev_op - 4
12:28 AlexDaniel joined 12:37 reportable6 joined
jnthn nine: The point of the check is indeed that the check didn't invoke anything 12:50
huh, wow, so we may have been missing out on an easy perf win 12:51
12:51 domidumont joined
jnthn That said, also one that wasn't common enough for me to notice in when analyzing spesh logs 12:51
*notice it 12:53
13:00 brrt joined 13:02 zakharyas joined
lizmat if someone here would have the time, I'd appreciate feedback on github.com/lizmat/P5wantarray/issues/1 13:04
MasterDuke nine: yeah, i was thinking a little more about subsequent templates. if i keep working down the list, should i just do a bunch at a time? 13:23
timotimo MasterDuke: keep in mind that the templates themselves are all more or less self-contained and we could just cut-paste mix-and-match however we like 13:39
MasterDuke true 13:45
nine MasterDuke: I'd commit them one by one for easier bisect 13:59
MasterDuke nine: definitely. but more than one in a PR? 14:21
nine MasterDuke: don't see an issue with that. We'll want to merge them quickly anyway. 14:23
MasterDuke k 14:24
14:56 brrt joined
nine Enabling logging of decont results doesn't seem to change the spesh result at all. Needs some further digging. 15:16
15:18 domidumont1 joined 15:35 brrt joined 15:59 MasterDuke joined
MasterDuke what does the ! mean on the end of some template names? 15:59
brrt it means that the template is destructive; that they do their own store operation to the result address, rather than have the expression compiler do it for them 16:02
the expression compiler can optimize it away, otherwise
MasterDuke ah. so `OP(sp_resolvecode): GET_REG(cur_op, 0).o = MVM_frame_resolve_invokee_spesh(tc, GET_REG(cur_op, 2).o);` would have `(template: sp_resolvecode! ...)` then? 16:04
oh, or when a register is passed to a function? 16:05
brrt - first would not
MasterDuke ok, thanks
brrt the basic version is MVM_foo_bar(tc, GET_REG(cur_op,2).o, &GET_REG(cur_op, 0));
when you pass the address to the fucntion 16:06
fwiw, i'm still working on the sp_findmethod one
MasterDuke did you see my PR? adding :invokish to the op makes everything work 16:07
github.com/MoarVM/MoarVM/pull/787 16:09
brrt lemmecheck 16:12
MasterDuke though no objection here if you have something better in mind 16:13
brrt hmm, i'm thinking.
what jnthn said about the special case not being worth it is true
having said that, this would always fallback to the negative case, because you're comparing the address of the spesh slot itself with the value of the stable 16:14
spesh_slot gives you it's address, not its value
MasterDuke ah, need to LOAD what it gives to do the comparison against? 16:15
brrt aye 16:17
also. i think this uncovers some bug in the register allocator, that i still want to investigate
and, we can cleanup the special-cased check in the emit.dasc version as well 16:18
furthermore, we can do the check a bit cheaper in this case. but it's not super relevant
becuase it's already the slow path
MasterDuke brrt: while you're here... what's wrong with this? `(template: sp_resolvecode (store $0 (call (^func &MVM_frame_resolve_invokee_spesh) (arglist (carg (tc) ptr) (carg $1 ptr))) ptr_sz))` 16:26
brrt you don't need that store 16:29
the store is automatically inserted for the non-destructive templates
MasterDuke ah, that's what you meant 16:34
same error though, node is too short 16:35
ah, missing the ptr_size at the end of the call 16:37
17:11 lizmat joined 17:41 zakharyas1 joined 17:43 evalable6 joined 17:49 lizmat joined
MasterDuke brrt: added the loads to the sp_findmeth template and updated the PR. rakudo builds ok, any easy way to be sure that path is being taken? 17:51
nine Fixing decont logging turns up 826: 293 x type Inline::Perl5::Interpreter (Conc) in the stats and the type of the object we call the method on seems to be known: Type 0: RW Scalar (Conc) of Inline::Perl5::Interpreter (Conc) 18:17
Yet the facts of the speshed code don't reflect that: "r30(4): usages=12, flags=4 Dcntd" and we still use sp_findmeth 18:18
18:18 lizmat joined 18:30 nebuchadnezzar joined 18:36 domidumont joined
nine Oh, the only way an object can get a MVM_SPESH_FACT_KNOWN_DECONT_TYPE is if it was passed as an argument. 18:36
19:13 lizmat joined 19:32 lizmat joined 19:36 quotable6 joined
timotimo well, that's not enough :) 19:44
19:59 lizmat joined 20:12 zakharyas joined, lizmat joined 20:15 zakharyas joined
jnthn timotimo: It actually *is* enough, without having things like alias analysis :) 20:17
We used to play a bit more fast and loose with that, and got bitten no small amount 20:18
nine: Suggest trying to add :logged here: github.com/MoarVM/MoarVM/blob/mast...plist#L444 20:20
timotimo oh, that's sensible indeed 20:46
21:02 Kaiepi joined
samcv if anyone wants to review the log github.com/MoarVM/MoarVM/wiki/ChangeLog-Draft 21:14
and let me know if any changes or reordering should be done 21:15
21:54 MasterDuke_ joined 22:22 squashable6 joined, unicodable6 joined, committable6 joined, bloatable6 joined
timotimo heeey, there's the one change i'm responsible for this month \o/ 22:24
MasterDuke likewise 22:26
huh. adding a template for box_i causes a failure in nqp's make test and the rakudo build breaks 22:27
but it's a pretty simple template
`(template: box_i! (callv (^func &MVM_box_int) (arglist (carg (tc) ptr) (carg $1 int) (carg $2 ptr) (carg $0 ptr))))` 22:28
jnthn Hm, wouldn't ! mean it's destructive? 22:30
timotimo it's true. callv also means void call, so that would fit
MasterDuke i thought it was since MVM_box_int takes the register
jnthn Oh 22:31
MasterDuke which i think is approximately what you said
jnthn yeah, was a bit surprised MVM_box_int does that
But if so, then yeah, destructive makes sense
MasterDuke `OP(box_i): { MVM_box_int(tc, GET_REG(cur_op, 2).i64, GET_REG(cur_op, 4).o, &GET_REG(cur_op, 0)); cur_op += 6; goto NEXT; }` 22:32
heh, that pasted way worse than it looked
jnthn Hm, then I don't spot anything obvious 22:33
Oh, hang on
Something can't be right
(carg $2 ptr) can't both mean the pointer value from the register and a pointer to the register 22:34
MasterDuke `compunitmainline requires an MVMCompUnit at gen/moar/stage2/NQPHLL.nqp:470 ` was the rakudo build error
jnthn I'd guess (carg $0 ptr) is the wrong one
And you need something that gets the address of the register $0
MasterDuke (carg $2 ptr) is GET_REG(cur_op, 4).o
jnthn: but that somehow "just works" for other templates 22:35
jnthn Something must represent the &
I'd be very surprised if that were magical
MasterDuke (template: smrt_strify! (callv (^func &MVM_coerce_smart_stringify) (arglist (carg (tc) ptr) (carg $1 ptr) (carg $0 ptr)))) 22:36
for `OP(smrt_strify): { /* Increment PC before calling coercer, as it may make a method call to get the result. */ MVMObject *obj = GET_REG(cur_op, 2).o; MVMRegister *res = &GET_REG(cur_op, 0); cur_op += 4; MVM_coerce_smart_stringify(tc, obj, res); goto NEXT; }` 22:37
check out atpos_i, that was a pre-existing one 22:39
does the same (carg $0 ptr) for &GET_REG(cur_op, 0) 22:40
jnthn Hmm 22:41
Maybe it's special-cased. brrt would know
Or any of us who reads the impl ;)
Or it may even be doc'd 22:42
MasterDuke i haven't noticed anything in the docs, but haven't done an exhaustive reading
22:44 Zoffix joined 23:08 domidumont joined 23:10 greppable6 joined
samcv jnthn: can you look at the change log and see if it needs any tweaking?github.com/MoarVM/MoarVM/wiki/ChangeLog-Draft 23:22
jnthn samcv: Looking through it and nothing stands out to me 23:26
samcv ok cool :)
jnthn samcv++
23:28 lizmat joined
Geth MoarVM: 76b09504ee | (Samantha McVey)++ | docs/ChangeLog
Update ChangeLog for release
 23:54
MoarVM: c522511e3a | (Samantha McVey)++ | docs/ChangeLog
A few ChangeLog adjustments
 23:55

Powered by Cro and the RakuĀ® Programming Language.

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

Thank you!