dalek arVM: e81b97d | (Jimmy Zhuo)++ | src/spesh/graph.h:
reorder struct in MVMSpeshOperand
07:24
arVM: ed63097 | (Jimmy Zhuo)++ | src/ (9 files):
avoid call MVM_callsite_get_common twice
arVM: 2bce4e5 | (Jimmy Zhuo)++ | src/core/coerce.c:
removed unused code
07:29
jnthn JimmyZ: e81b97d is senseless, not to mention weird 08:03
The notation in the literature is always orig with i as a subscript.
nwc10 jnthn: not knowing that, it looks initially reasonable, if one assumes that a reasonable approach to avoiding alignment holes on structures is to order them with biggest things first (unless some other reason pre-empts that) 08:05
also, why is SSA-computed version signed? 08:06
jnthn nwc10: Yeah but there are 64-bit things in the union. 09:00
So the hole is not making things any worse.
Signed? Good question, quite possibly because -1 is used as a sentinel...or was going to be. 09:01
nwc10 not sure whether one ends up with more bugs as a result of negative values accidentally becoming array subscripts, or unsigned "all bits set" not being transfered "correclty" to larger integer types 09:04
jnthn I should check if it ended up using the -1 in such a way 09:05
If not it can become unsigned
JimmyZ jnthn: Do I still need to revert it? 09:09
jnthn It just feels like a really pointless patch :/
It achieves nothing.
Except wasting my time reviewing it.
JimmyZ sorry :( 09:10
jnthn ed6309714f is sensible overall, though I'd have preferred to see the various extracting of tc->cur_frame to a local in a separate patch. 09:18
In most cases it's "obviously correct", but in some places ti's in code that is messing with which frame is the current frame. I think it's correct where it's been done, however. 09:19
JimmyZ jnthn: Thanks for review. I did take a care of right time about extracting of tc->cur_frame. 09:29
jnthn Yeah. It's one of those times where it's borderline whether it's better to extract it, or to do the "obviously correct" thing and leave the compiler's CSE to try and take care of it. 09:30
(In the unwind/exception run cases where such a refactor could be delicate, I mean) 09:31
JimmyZ I objdumped libmoar.so and found gcc doesn't do CSE to tc->cur_frame->xxxx = yyyy; the tc->cur_frame part. at least O1 doesn't 09:34
-O1 09:35
so I did some extracting
TimToady m: for <aa aba> xx 50 { if not m/b/ { print "."; next }; print "!"; }; say ""; 19:16
camelia rakudo-moar 9c74ab: OUTPUT«.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!............................................␤»
TimToady spesh bug 19:17
but not jit, inline, or osr
removing the not fixes it 19:19
changing the control flow to if/else fixes it
using unless instead of 'if not' fixes it
it's quite picky :) 19:20
well, you can change the 'not' to '!' and it still breaks, anyway 19:21
m: for <aa aba> xx 50 { if m/b/ { print "."; next }; print "!"; }; say ""; 19:22
camelia rakudo-moar 9c74ab: OUTPUT«!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.␤»
TimToady m: for <aa aba> xx 50 { if not not m/b/ { print "."; next }; print "!"; }; say "";
camelia rakudo-moar 9c74ab: OUTPUT«!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!␤»
TimToady hmm 19:23
m: for <aa aba> xx 50 { if not not not m/b/ { print "."; next }; print "!"; }; say "";
camelia rakudo-moar 9c74ab: OUTPUT«.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!............................................␤»
TimToady m: for <aa aba> xx 50 { if not not not !m/b/ { print "."; next }; print "!"; }; say "";
camelia rakudo-moar 9c74ab: OUTPUT«!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!.!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!␤»
timotimo ducks 20:33
FROGGS quacks 21:01
lizmat doctor 21:02
tgt TimToady: Reverting github.com/rakudo/rakudo/commit/66...8966382ef2 fixes that issue. No idea why, I just let git bisect lose on it. 21:36
timotimo i KNEW it'd be my fault :) 21:37
lizmat FWIW, I would think only the Mu candidates need reverting 21:51
timotimo: something should have ticked you off, because you could optimize that further by removing the Bool candidates :-) 21:52
timotimo oh
lizmat is a great fan of lining out similar code 21:53
you would have seen it then
jnthn I'd probably not revert the Rakudo patch if it's clearly a VM-level issue 22:02
Should fix the spesh bug instead. 22:03
lizmat fg 22:04
oops 22:05