freenode/#clasp - IRC Chatlog
Search
16:48:18
kpoeck
drmeister I meant something like in the comment to https://gist.github.com/drmeister/44c864a2cfc959ee404bfc17ab60991a
16:50:06
drmeister
Yes - for this part - I agree - let's get it working with encode/decode . But I'd like to understand what is going on. But please proceed.
16:53:42
kpoeck
What I was trying to do is the equivalent of https://github.com/s-expressionists/Eclector/blob/wip-reader-implementation-documentation/code/reader/fixup.lisp#L28-#L32
16:54:32
drmeister
There are almost no 'fields' methods in clasp - so I don't know how to put together an example outside of cando - so... kpoeck it's good you have cando building.
16:55:02
drmeister
kpoeck: Cando adds about 150 C++ files and lots more C++ classes/functions/methods.
16:55:41
kpoeck
but going on, once we have the class, we need to iterate over the slots&values of the instance to eventually patch up the slots
16:55:45
drmeister
Yeah but core::cxx-objects is just the base class - it doesn't implement ::fields itself. It's up to each class to implement its own fields method.
16:56:50
drmeister
We can't iterate over the slots&values of C++ instances - they are too low level to do that. You can iterate over the entries of the alist returned by (core:encode object) and then you can write that rewritten alist using (core:decode object new-alist)
16:57:38
kpoeck
Could you look at my comment on https://gist.github.com/drmeister/44c864a2cfc959ee404bfc17ab60991a
16:58:52
drmeister
Ah - I have to click on it. icloud shows the first lines and I thought that was all there was.
17:01:03
Bike
drmeister: if you want to do it without consing, what you could do is expose a generalized patcher kind of C++ method, that iterates over the slots (in whatever sense), calls a given lisp function on each value, takes the return value from that function, and puts it in the slot
17:04:42
drmeister
Right (1) get the alist using (core:encode object) (2) fix up all the values with rplacd (3) stuff the values back into the object with (core:decode object fixed-up-alist)
17:05:45
kpoeck
Perhaps the recursion for (fixup client value-fixed seen-objects mapping) is missing
17:07:38
Bike
(rplacd slot-and-value (if found-p (progn (setf fixed-up t) value) (fixup client value-old ...))) or something
17:08:52
kpoeck
did a new version n https://gist.github.com/drmeister/44c864a2cfc959ee404bfc17ab60991a
17:10:22
drmeister
There's a typo value-fixed is never defined? Should it be 'value'? or rename 'value' to 'value-fixed'
17:12:46
drmeister
Also, if fixed-up isn't set to T (none of the alists values needed fixing up directly) but the recursive (fixup ...) did fix something up. Then we still need to call (core:decode ...)
17:12:52
Bike
in the analogous standard-object method it uses current-value, which is the value passed to gethash
17:13:27
Bike
like, the cxx object will have a cons in it, and fixup will modify the cons, but the cxx object just refers to the same cons
17:13:53
drmeister
So we only call (core:decode object new-alist) if the alist in the loop is modified.
17:15:18
drmeister
No worries kpoeck. This is difficult stuff and we are all working towards the correct solution.
17:16:58
kpoeck
Updated version in https://gist.github.com/drmeister/44c864a2cfc959ee404bfc17ab60991a
17:17:11
drmeister
I spend plenty of time working by myself with just my demons. It's always a joy to solve a problem with you guys.
17:19:23
drmeister
Ok. Now isn't there something where if the (not (eq value value-old)) then we know that there was no substitution. Could this be more efficient if we were using this property of value/value-old?
17:19:58
Bike
Don't we just know whether there's a substitution from whether there's something in the hash table?
17:21:11
drmeister
Bike: I agree - it may be all we need to know that there was something in the hash-table - that found-p is T.
17:21:58
drmeister
Ok, on running fixup on 'value' - does that happen somewhere else? Or do we have to do it here?
17:22:40
Bike
Yes, we have to do it here, because every component of every compound object needs to have fixup run on it once.
17:23:47
kpoeck
In general fixup is called from here: https://github.com/s-expressionists/Eclector/blob/wip-reader-implementation-documentation/code/reader/read.lisp#L28
17:23:55
Bike
if you're worried about efficiency i already told you what we could do to avoid the consing of encode
17:24:36
kpoeck
drmeister does this change - although perhasp consing to much - helping the error you had?
17:26:14
scymtym
Bike: i think the complete "seen" table is unavoidable since you could stick #.*my-pre-existing-circular-object* into the input
17:27:08
drmeister
Bike: your suggestion to reduce consing was... "drmeister: if you want to do it without consing, what you could do is expose a generalized patcher kind of C++ method, that iterates over the slots (in whatever sense), calls a given lisp function on each value, takes the return value from that function, and puts it in the slot"
17:28:48
scymtym
Bike: i was referring to the necessity of putting every object in the SEEN-OBJECTS table in FIXUP
17:30:35
drmeister
I think a place to put this would be... src/lisp/kernel/cleavir/activate-clasp-readtables-for-eclector
17:33:09
drmeister
Bike: To follow your suggestion I could add a new mode to Record_O called 'eclector_patching' - then I would add the callback to Record_O and modify all code that implements 'case patching:' to add 'case eclector_patching:' and that code would do what you say.
17:33:39
drmeister
There would be other effects though - we might need to rewrite the other clasp specific eclector.reader:fixup methods for hash-table for instance.
17:34:48
Bike
eclector has a built in fixup method for hash tables, right? that just uses gethash and (setf gethash)?
17:35:55
drmeister
Maybe not - I'm tossing that out there and then you challenge me and then I dig deeper.
17:36:23
Bike
i mean i dont understand what you're thinking? "rewrite the other clasp specific eclector.reader:fixup methods"? but we have no specific methods
17:37:00
drmeister
Clasp has this: https://github.com/clasp-developers/clasp/blob/dev/src/core/hashTable.cc#L602
17:37:35
drmeister
Ok, good. It's not even consistent. https://github.com/clasp-developers/clasp/blob/dev/src/core/hashTable.cc#L628
17:37:35
Bike
and as you can see, we're not even using it for clasp's native ##, since the patching: case just crashes clasp
17:38:04
drmeister
https://github.com/clasp-developers/clasp/blob/dev/src/lisp/kernel/lsp/sharpmacros.lsp#L32
17:39:48
kpoeck
I'd say teh coed you just pasted is no longer used after bootstrapping and we handle it here
17:39:51
kpoeck
https://github.com/s-expressionists/Eclector/blob/wip-reader-implementation-documentation/code/reader/fixup.lisp#L34-#L41
17:44:59
drmeister
I'll check if the method is making any difference by restarting cando and trying it without it.
17:46:26
drmeister
Ok, it's having some effect. I'm seeing something different. Hang on - I'll make it print out more info.
17:51:19
drmeister
The atom in the bond in C1 is being fixed up properly. But the bond object in C1 should also be used to fixup the bond object in C2 - that is not happening.
18:06:05
drmeister
Let's take it slow. Here is a new change I made to the method to get more insight...
18:08:04
drmeister
It doesn't find a fixup - of course not - it's a vector containing (T) and this is all that is in the mapping...
18:09:46
drmeister
https://github.com/cando-developers/cando/blob/dev/include/cando/chem/atom.h#L202
18:10:05
drmeister
VectorBond... https://github.com/cando-developers/cando/blob/dev/include/cando/chem/atom.h#L167
18:10:49
drmeister
https://github.com/clasp-developers/clasp/blob/dev/include/clasp/core/record.h#L139
18:11:20
drmeister
So Clasp's Record_O feature needs to know what is going on. encode/decode can't work without support from Record_O.
18:12:29
drmeister
Ok. But fixup is going to have to tell the caller that it fixed something up - correct?
18:13:18
Bike
the problem is that fixup will alter this constructed lisp vector, but the vector isn't actually what's in the cxx object.
18:14:24
Bike
it means the encode/decode won't actually work, since clasp isn't actually able to represent the slot value as a lisp object.
18:15:15
drmeister
What I need to do is change how Record_O works. I need to make it so that it can use what Eclector passes to eclector.reader:fixup
18:15:58
Bike
well, it could still work with what i said with the one-arg function, it would just have to be more involved
18:16:19
drmeister
Damn, I couldn't put my finger on what was wrong - but my subconscious has been screaming at me for the past couple of hours.
18:17:24
drmeister
No - I don't think we can avoid it. I have data structures that aren't exposable to Common Lisp.
18:18:53
drmeister
kpoeck: The problem is I have data structures in the C++ code that aren't/can't-be exposed to Common Lisp.
18:19:14
drmeister
I could get rid of them - and use only Common Lisp data structures - but that would require some work.
18:19:48
drmeister
Like - in this case - a SimpleVector_O containing Bond_sp could work without too much trouble.
18:20:18
drmeister
But I'd have to initialize it in a different way that I do now and we would have to change the code that touches the Atom_O::bond slot
18:24:11
Bike
i don't think you should change how all the classes are laid out. besides being a lot of work, it would be nice for this all to work with a lot of different C++ code.
18:24:49
drmeister
I can change the 'patching' mode so that it doesn't call record_circle_subst directly, but it calls a closure.
18:26:48
drmeister
What if we have Record_O take a function in patching mode and that function takes two arguments - a 'mapping' and the object and it returns the object or another object from the mapping?
18:29:44
drmeister
https://github.com/clasp-developers/clasp/blob/dev/include/clasp/core/record.h#L171
18:35:15
drmeister
core:make-record-patcher adds an optional argument for the callback and the default value is a trampoline to record_circle_subst
18:45:49
drmeister
Maybe - I'll give it a try with this and we will see. The Record_O::fieldxxx template functions and core:circle-subst handle recursion properly.
18:48:49
drmeister
That calls core:circle-subst. I'd need to substitute a different function. What would that different function do?
18:50:16
kpoeck
Could you please also have a look at https://gist.github.com/kpoeck/d1b2df2d670dc42e1cdc3f4e8144682b and see whether this looks fine?
18:50:53
kpoeck
I meant the result in https://gist.github.com/kpoeck/d1b2df2d670dc42e1cdc3f4e8144682b
18:51:52
drmeister
kpoeck: The problem is going to be in the C++ - the field on line 3 will not work properly.
18:53:15
drmeister
A Vec0<Bond_sp> is not representable in Common Lisp - so a simple-vector is created to represent it. But they aren't identical objects so the fixup will fail.
19:28:39
Bike
having the record patch just take a closure lets you do literally any operation at all, so i don't see the point of having a mapping
21:18:05
paulapatience[m]
Ok, so I'm trying to build a debug version of clasp with ./waf build_cboehm_d, but I get a compilation error. (I didn't distclean the directory since my previous (successful) compilation of nondebug clasp; should I have?)
21:18:05
paulapatience[m]
My changes to wscript, my wscript.config, and the compilation log (with -v): http://ix.io/2gFM
21:20:36
paulapatience[m]
Ok I figured the undefined functions might be the problem, but it's true that no other error is mentioned
21:22:02
paulapatience[m]
Perhaps I'll try compiling from a clean checkout then. But I am right in assuming that ./waf build_cboehm_d builds the debug version?
21:24:01
drmeister
./waf build_cboehm_d does build the debug version. It's quite slow and I hardly use it because it is so slow.
21:24:16
paulapatience[m]
(I don't have the lldb/assembly chops to debug the -O3 optimized release version)
21:25:57
paulapatience[m]
I'm going to have to go to the store for some groceries but I'll be back in a bit
21:43:26
drmeister
It's not even necessary - it's lazy initializers failing that were initialized when clasp started.
22:06:34
paulapatience[m]
So it would be a Linux-only problem? (though I seem to recall kpoeck having the same problem on macOS)
22:58:18
drmeister
I'm not sure. On linux it's a static constructor that shouldn't be needed but is being invoked because I'm including clasp headers that declare it.
22:58:57
drmeister
Static constructors are getting a big refresh by llvm11 - but that's not ready yet.
22:59:16
drmeister
Another idea would be to compile and link the code with Clasp and load the dynamic library.
23:00:45
paulapatience[m]
Compile and link the cxx code with clasp rather than clang, you mean? Clasp can do that?
23:03:44
drmeister
Right now we are taking a short-cut. We are compiling the demo code to bitcode. The .bc file in each directory
23:05:25
drmeister
Here we are kind of spoofing that. We are compiling a C++ file to bitcode and making it look like a clasp generated bitcode file. But it's not a perfect spoof because we compile C++ header files that are declaring static initializers.
23:06:19
drmeister
When we load the bitcode into clasp it compiles the bitcode to native code and links it against symbols that are already loaded.
23:07:02
drmeister
Maybe it's the code model. Maybe the symbols are more than 2GB away and they can't be linked?
23:07:36
drmeister
The linux JIT linker is being rewritten as we speak. By llvm11 this will be a lot easier.
23:07:54
drmeister
They are making the JIT linker work very much like the system linker. Right now everything is hacked together.
23:08:26
drmeister
That's my understanding. But it's informed by long discussions with Lang Hames - the developer of the new ORC JIT.
23:13:04
paulapatience[m]
ACTION sent a long message: < https://matrix.org/_matrix/media/r0/download/matrix.org/umdQtPUqjIZDjoLWKCOVkbUd >
23:16:08
paulapatience[m]
what command did you run to get that output? I can't seem to get the disassembly around frame #1
23:18:59
drmeister
It's putting 0x0 in %rax and then calling it two instructions later - that's crazy. So it makes me think the $0x0 should be an address that failed to link.
23:20:20
paulapatience[m]
I naively assumed that since we compile clasp with llvm we should debug it with lldb
23:20:28
drmeister
There's nothing wrong with calling 0x0 - until you get there and hit a page fault.
23:21:01
drmeister
Sure - but lldb needs to be installed on linux and I often don't take the trouble.
23:23:09
paulapatience[m]
I used to use gdb a lot when debugging microcontrollers a few years ago, but it's the kind of stuff you forget from lack of use...
23:25:33
drmeister
That's why I think the problem is linking. The linker should be putting an address in there.
23:35:49
drmeister
call void @_ZN3reg7type_idC2ERKSt9type_info(%"class.reg::type_id"* %1, %"class.std::type_info"* dereferenceable(16) bitcast (i8** @_ZTIN4core6Real_OE to %"class.std::type_info"*))
23:35:57
drmeister
Followed by %5 = call i64 @_ZN3reg17allocate_class_idERKNS_7type_idE(%"class.reg::type_id"* dereferenceable(8) %1)
23:40:43
paulapatience[m]
Ok those are the functions that we should be calling but that are called as 0 instead?
23:42:11
drmeister
There are only two calls, the first one seems to be to a reasonable address, the second one to 0x0
23:47:46
drmeister
Here's something interesting. When I compile it to an object file. A bunch of __cxx_global_var_init.xxx functions don't appear to generate code.
23:56:21
paulapatience[m]
I'm getting 0s for a bunch of other functions too, though. Is that normal?
0:01:49
paulapatience[m]
but the cxx_global_var_init ones are in the text section so I would imagine they don't get overridden. but i do not have much experience in this.
0:02:39
drmeister
I disassemble the startup function and it's full of 0x0 - it isn't linking at all.
0:06:41
drmeister
Lines 9, 14, 54 - You can search for $0x0,%rax and then see a call to *$rax right after that.
0:21:22
drmeister
I think the linking on linux is going to be broken until the JITLinker is completed for linux in llvm11
0:22:10
drmeister
I had this stuff working on linux at one point - but it's the hardest stuff to keep working - even though it's a signature feature of clasp.
0:23:42
drmeister
When we load bitcode - it needs to link the addresses in the native code generated from the bitcode to symbols in the executable - yes.
0:24:09
drmeister
We could use the system linker - but then we need to generate a clasp dynamic library to link against - we don't have that. We generate a clasp executable.
0:51:46
paulapatience[m]
You mentioned that the static initializers are already initialized when we call runStaticConstructorsDestructors (if I understood correctly). Would it be possible to disable the reinitialization somehow?
1:28:15
drmeister
No - because our static initializer is in the list of static initializers. It's just that we picked up a bunch more from header files.
2:31:12
Bike
fixup is called for effect, so it just needs to call fixup on value-old before returning it
2:47:52
Bike
you could just pass (lambda (object) (circle-subst object mapping)) to make-record-matcher