freenode/#clasp - IRC Chatlog
Search
15:42:18
drmeister
Bike: I'm hitting a problem with characters and I need to check the inlined code for characterp - can you point me to where that happens?
15:43:20
drmeister
It's unlikely that anything is wrong but I'm getting an illegal character in the byte-code of quicklisp package.lisp
15:43:49
drmeister
It has the tag #B1010 and that is illegal - since we switched to 4-bit tags I need to check it.
15:44:12
drmeister
When I switched to 4-bit tags I changed the character shift to 4 bits and #B0010 is the character tag.
15:44:18
Bike
yeah, characterp is turned into typeq character. then it'll be hir-to-mir'd into a characterp instruction. the translator for that calls compile-tag-check which is here: https://github.com/clasp-developers/clasp/blob/master/src/lisp/kernel/cmp/typeq.lsp
15:47:17
Bike
https://github.com/clasp-developers/clasp/blob/master/src/lisp/kernel/cleavir/translate-instruction.lisp#L841-L845 it uses immediate-mask and character-tag
15:48:46
drmeister
FYI +immediage-mask+ is now 15 and there are now 4 (four) FIXNUM tags. Character and small-float shifts are 4 bits.
15:49:20
drmeister
Does that turn on any warning bells with you? Do you need to change any of the typeq or predicate or fastgf code to handle this new scheme?
15:49:45
Bike
i think it's all in terms of constants defined in cmpintrinsics and stuff, so it ought to be fine
15:49:47
drmeister
We have everything in constants and if you are using them properly it should all work out when I change the tagging scheme like this.
15:50:39
Bike
maybe you could look for uses of the irc- for shift instructions to make sure they're all in terms of constants?
15:52:13
Bike
"no viable overloaded '='" "note: candidate function (the implicit copy assignment operator) not viable: 'this' argument has type 'Monomer_O::Couplings::const_iterator' (aka 'const GCVector_moveable_iterator<bla bla bla>'), but method is not marked const
15:52:29
Bike
i can paste the full error if that's not descriptive enough. ti's from a few places in cando
16:00:18
drmeister
You should be hitting this - it's a sanity check and I changed the name of the _Length slot to _MaybeSignedLength and didn't even bother to try and compile it. I just pushed it to github.
16:00:32
drmeister
https://github.com/clasp-developers/clasp/blob/master/include/clasp/gctools/gcarray.h#L43
16:01:46
drmeister
I think the times on github are when I commit the change, not when I push - right?
16:09:40
drmeister
Ok, when I changed _Length to _MaybeSignedLength - I did that to catch exactly what I'm seeing now.
16:10:10
drmeister
We expose the offset of _Length to Common Lisp. We need to make sure that we aren't using the offset without taking the absolute value.
16:11:55
drmeister
I'm working on the principle that if we read the length of an array that we will have it in a register and we can immediately take the absolute value of it at an insignificant cost.
16:13:38
drmeister
In most cases we won't need to take the absolute value however. We can just keep going like we have been.
16:15:14
drmeister
Ok, here's the places were we directly or indirectly expose the offset of the _MaybeSignedLength slot.
16:16:40
drmeister
I think we don't need to make any changes. In none of these cases would we use this information to read the _MaybeSignedLength slot and find a negative value in there that would need to be converted to a positive value - correct?
16:17:30
Bike
well, i still think it's kind of annoying to alter this fundamental array structure just to save a byte in bignums
16:18:28
drmeister
And it's how GMP works - they use a negative length to represent a negative value.
16:20:01
Bike
we don't actually have to follow gmp in this aspect. none of the mpn functions deal with negative numbers really
16:21:07
Bike
that's a lot of what the mpz functions do. like, if you want to add a positive number x and a negative number y, you have to do x-abs(y) instead, basically
16:21:12
drmeister
We are kind of halfway down this road already though. It only impacts the code in two places... 1. When initializing a GCArray_moveable and 2. In MPS obj_scan. In those two places I need to take the absolute value.
16:21:56
Bike
it's probably not a big deal for performance or anything, i'm just annoyed when changes in one part of the code require changes in some basically unrelated part
16:23:12
Bike
i mean it's kind of the same thing with the mpzs that are in the code everywhere. i was hoping i'd just be able to change bignum.h/cc, but that's impossible since huge parts of the system assume bignums have an underlying mpz and work with it directly
16:23:52
Bike
i don't mind going on with the signed length thing, i just wish we could localize it more is all i gues
16:28:08
drmeister
MPS changes this.... size_t capacity = *(size_t*)((const char*)client + stamp_layout.capacity_offset);
16:28:33
drmeister
To this... size_t capacity = (size_t)std::llabs(*(int64_t*)((const char*)client + stamp_layout.capacity_offset));
16:42:15
drmeister
Do you feel really strongly about this? Like on a scale of 0 to 10 where 0 is don't care at all and 10 is this is the worst idea ever?
16:42:33
drmeister
We can add a word to TheNextBignum_O to represent the sign and I can unwind these changes.
16:43:17
drmeister
Because in the balance it has been a more complex change than I thought. I'm kind of at a 6 on that scale but the changes are all done now.
16:46:32
Bike
the signed length thing is fine, i just want that to affect as little of the rest of the system as possible
17:39:31
Bike
drmeister: how can i write sxhash_ for next-bignums, by the way? is there some way to just feed in the limbs?
17:40:55
drmeister
https://github.com/clasp-developers/clasp/blob/master/include/clasp/core/object.h#L235
18:58:18
Colleen
kpoeck: drmeister said 22 hours, 8 minutes ago: Yes - I finished merging into master.