Welcome to the main channel on the development of MoarVM, a virtual machine for NQP and Rakudo (moarvm.org). This channel is being logged for historical purposes.
Set by lizmat on 24 May 2021.
00:08 reportable6 left 00:09 reportable6 joined
Geth MoarVM: MasterDuke17++ created pull request #1682:
Add missing _u cases to disp program callsite code
MasterDuke vrurg: ^^^ 00:13
off to sleep
vrurg MasterDuke: would wait until the next bump, perhaps. 00:33
02:24 harrow left 02:30 harrow joined 03:41 evalable6 left, linkable6 left 03:42 evalable6 joined 04:42 bloatable6 left, benchable6 left, squashable6 left, evalable6 left, coverable6 left, nativecallable6 left, sourceable6 left, statisfiable6 left, committable6 left, unicodable6 left, shareable6 left, notable6 left, bisectable6 left, releasable6 left, tellable6 left, reportable6 left 04:43 releasable6 joined 04:44 bisectable6 joined, reportable6 joined, squashable6 joined, unicodable6 joined 05:43 tellable6 joined, notable6 joined 05:44 linkable6 joined, sourceable6 joined, shareable6 joined, committable6 joined 05:45 benchable6 joined 06:43 evalable6 joined, nativecallable6 joined 06:44 bloatable6 joined 06:45 statisfiable6 joined 07:45 coverable6 joined
Geth MoarVM: b0723bcb29 | (Daniel Green)++ | src/spesh/disp.c
Add missing _u cases to disp program callsite code

Fixes the `MoarVM panic: Unknown dispatch op when resolving callsite` in HTTP::HPACK.
MoarVM: a1a45e57ed | niner++ (committed using GitHub Web editor) | src/spesh/disp.c
Merge pull request #1682 from MasterDuke17/add_some__u_cases_to_disp_program_callsite_code

Add missing _u cases to disp program callsite code
08:11 Shane__ left 08:21 frost joined 09:20 discord-raku-bot left 09:31 discord-raku-bot joined 09:39 discord-raku-bot left, discord-raku-bot joined
jnthnwrthngtn moarning o/ 10:09
jnthnwrthngtn just spent a minute looking for his coffee cup...before realizing it was on his desk, full of coffee, made 2 minutes beforehand... 10:10
gfldex jnthnwrthngtn: sounds like you really need a cup of coffe right now 10:11
nine Ah the dreaded coffee bootstrap 10:13
10:22 Geth left, RakuIRCLogger__ joined, TempIRCLogger left, Geth joined 10:23 lizmat left, RakuIRCLogger left, lizmat joined 10:24 TempIRCLogger joined 10:27 RakuIRCLogger__ left, RakuIRCLogger joined
Nicholas \o 10:31
today at the hotel breakfast some poor lady had a coffee bootstrapping problem - before first coffee, didn't correctly work out which sized cup to put under the coffee machine
flood and panic. She felt bad. I said I'd goofed this sort of thing before, but got it right at this hotel. 10:32
MasterDuke huh. i was looking at en.algorithmica.org/hpc/algorithms/gcd/ and github.com/lemire/Code-used-on-Dan...26/gcd.cpp and if i add sergey's version to lemir's benchmark and run it, the sort of naive github.com/lemire/Code-used-on-Dan...pp#L38-L64 is 10:38
noticeably fastest
but it's quite a bit slower when put in moarvm
jnthnwrthngtn Different optimization flags, maybe? 11:00
MasterDuke the flags are pretty similar. might be size of variables, trying to make them consistent with what moarvm uses now
will double check flags next 11:01
hm, it is c++ vs c. but there aren't very many c++ features being used... 11:03
moon-child iirc there are a couple of things that are ub in c++ but not in c 11:08
unbounded recursion, and some usage of unions ... probably doesn't apply here 11:09
MasterDuke weird. changed all the sizes, timings in the benchmark stayed the same. put a different one from the benchmark that was also faster, but in moarvm it was slower 11:18
i even tried putting in one version that the benchmark said was slower, in case it was actually backwards. but nope, slower than stock in moarvm also! 11:39
hm, it's very cpu architecture dependent. on my haswell laptop, sergey's version is much faster, ~2.5s instead of ~4.6s for master. on my zen2 desktop, sergey's version is ~2.1s instead of ~2.0s for master. both using gcc 11.2.0 12:02
ok, this isn't usually hot code so i'm not going to bother trying to put in a tuning step during moarvm building to choose different versions. experiment finished 12:09
12:27 evalable6 left, linkable6 left 12:56 Kaipi left 12:59 Kaipi joined 13:08 frost left 13:27 evalable6 joined 13:30 linkable6 joined 13:46 frost joined 14:07 frost left
MasterDuke isn't the ternary unnecessary here? github.com/MoarVM/MoarVM/blob/mast...rap.c#L374 15:27
because the result should be statically initialized to 0 when created, right? github.com/MoarVM/MoarVM/blob/mast...rgs.c#L600 15:30
oh hm, according to a quick test on godbolt, just declaring one won't initialize it to 0, but setting the MVMString * part of the union to NULL will github.com/MoarVM/MoarVM/blob/mast...rgs.c#L567 15:34
huh, it's zero after just the declaration at '-O3', but not at '-O0' 15:35
which means we can change MVM_args_get_named_int to actually return an int, not the struct, and then param_rn_i turns into just a function call and can be easily jitted 15:41
am i missing anything? 15:42
and for the str and obj version, it looks like everywhere .exists on the result is checked, the value would be NULL when !.exists and not-NULL when .exists, so we can instead just check if the value is NULL 15:47
15:49 evalable6 left, linkable6 left, evalable6 joined 15:51 linkable6 joined
nine Stack variables are uninitialized by default in C. If they happen to be 0 that's just a coincidence. 16:00
MasterDuke ok, but setting the union to NULL will always zero them, correct? 16:03
nine exists is not in a union 16:05
MasterDuke yep, but here `result.arg.s = NULL;` arg is the union 16:06
nine But how does that help us avoid returning a struct? 16:07
MasterDuke because the default value that's set everywhere that .exists is checked is 0, or something not-NULL. i think we still need .exists for other things, but for setting a default value we can check against the value being NULL for the objects and just pass through the value for the primitives 16:11
hm, github.com/MoarVM/MoarVM/blob/mast...1090-L1096 is going to be a problem 16:21
well, the positionals have both `*_required_*` and `*_optional_*` version, where the required do return the type they're looking for and the optional return an ArgInfo 16:23
guess that's what would actually have to be done
ah, so currently we don't jit any optional param ops or named param ops, just required positional. adding required named wouldn't be too bad (just splitting the current MVM_args_get_named_*` into required and optional versions 16:33
but it's the optional ones, both positional and named that are trickier 16:34
how much benefit is there to jitting these param ops? 16:37
oh, looks like spesh can change them into sp_getarg_* (so no benefit to jitting when that happens) 16:44
nine Yes, argument speshing is way better than just JITing. 17:01
MasterDuke yeah, maybe this is another experiment that's not really going to be worth the time 17:02
however, i'm seeing a lego jit bail in find_method because of param_on_o 17:04
this one github.com/rakudo/rakudo/blob/mast...nqp#L8-L35 17:05
and i'm not sure how bad that is
nine The first question is always: "why does arg spesh fail?". The answer is in the spesh log 17:09
MasterDuke       # [000] bailed argument spesh: unhandled coercion 17:10
      param_on_o        r3(2), lits(no_fallback),   BB(3)
nine Oh....but which case? 17:25
In any case it seems to lack support for param_rn_u and param_on_u 17:27
lizmat jnthnwrthngtn: mta.openssl.org/pipermail/openssl-...00216.html
jnthnwrthngtn lizmat: I figure since it's an update to a shared library most folks will get such updates via their OS package managers; the exception is Windows where the OpenSSL module bundles a DLL, but I'm not the maintainer of that module. 17:32
Worth making an issue there to request the Windows DLL is updated. 17:33
lizmat jnthnwrthngtn: I guess... but maybe an update to the OpenSSL modules could ensure an up-to-date OpenSSL is used ?
jnthnwrthngtn If there's a new release of the Raku OpenSSL module with said DLL I can ship an IO::Socket::Async::SSL that requires a minimum version. I don't believe there's much the OpenSSL module itself can do otherwise since it's a native dependency? 17:34
nine Ensuring that such libraries are up to date is the operating system's job 17:35
lizmat nine: but we could help with that, no?
jnthnwrthngtn How?
lizmat I assumed there was a way to interrogate the library for its version 17:36
if the version is too low, decided on some action ?
nine Why should we? OpenSSL is used by hundreds of software packages. I would positively hate them all to tell me about some OpenSSL update 17:38
lizmat ok.... :-) 17:41
nine To expand on that: OpenSSL is managed by my operating system. I fully expect to get the security update installed automatically in the next few days. There's nothing I have to do and until the update is available, there's nothing I can do. It would not help me and only harm me if I e.g. tried to install Cro and can't because the OpenSSL Raku module is refusing to run. 17:43
lizmat understood :-) 18:02
18:19 discord-raku-bot left 19:07 ShaneC joined 19:15 [Coke] left 19:17 [Coke] joined 19:31 discord-raku-bot joined
MasterDuke nine: it's this case github.com/MoarVM/MoarVM/blob/mast...#L654-L660 and found_flag is MVM_CALLSITE_ARG_INT 19:47
nine MasterDuke: sounds like a native int is passed in $no_fallback when we expect an object 20:13
MasterDuke yeah, looks like this fixes it: 20:14
             case MVM_OP_param_on_o:
-                if (found_idx != -1 && !(found_flag & MVM_CALLSITE_ARG_OBJ)) {
+                if (found_idx != -1 && !(found_flag & MVM_CALLSITE_ARG_OBJ)
+                        && !(prim_spec(tc, type_tuple, found_flag_idx) == MVM_STORAGE_SPEC_BP_NONE)) {
nine Wait...are you saying that due to a bug we never actually entered this case? github.com/MoarVM/MoarVM/blob/mast...gs.c#L1033 20:17
MasterDuke don't know 20:18
nine It looks like that. Does it pass a spectest with blocking and nodelay? 20:24
MasterDuke passes a spectest, didn't try with blocking and nodelay
nine blocking and nodelay are always a good idea when working on spesh
MasterDuke that case does get hit a couple times, but always ends up in the found_idx == 1 branch 20:25
nine Not very surprising. Optional names are probably omitted a lot 20:27
20:28 ShaneC left, ShaneC joined
MasterDuke fyi, i had my prim_spec patch live when i saw that case hit, haven't tried yet without (currently running another spectest) 20:29
and oh wow, it's slow. m-test took 99s instead of 22s 20:30
passed 20:45
m: sub a(:$a) { $a ?? $a + 1 !! 0 }; my int $a = 1; for ^10_000_000 { $a = a(:$a); }; say now - INIT now; say $a;
camelia 1.770201144
MasterDuke locally that drops from ~1.66s to 1.55s with MVM_SPESH_BLOCKING=1 20:46
nine very nice :) 20:56
Geth MoarVM: MasterDuke17++ created pull request #1683:
Fix speshing of param_on_o when we can coerce the arg
MasterDuke hm. i actually added that case in my recent PR because i noticed it was missing. but i guess the omission was intentional, because everything should be able to coerce to obj, right? 21:07
nine which case? 21:08
MasterDuke maybe github.com/MoarVM/MoarVM/blame/mas...#L654-L660 21:09
maybe it'd be better to just remove it entirely? or leave it in with just a comment and a break? 21:10
nine Ooooh...that explains it! Well yes, then that case is superfluous 21:17
Geth MoarVM: 0b45dd48bb | (Daniel Green)++ | src/spesh/args.c
Fix speshing of param_on_o

This case was added in github.com/MoarVM/MoarVM/pull/1681, but isn't necessary and causes spesh to not turn it into an sp_* op, and then since it can't be jitted either it was causing a bail. So remove the body of the case, but leave it in so nobody else thinks it was accidentally left out.
MoarVM: 588e00d60e | MasterDuke17++ (committed using GitHub Web editor) | src/spesh/args.c
Merge pull request #1683 from MasterDuke17/correctly_spesh_param_on_o
23:41 leont left 23:42 leont joined 23:43 AlexDaniel left 23:56 AlexDaniel joined