Re: [PATCH] Add debug stream parameter to init_jit()

[Paul Cercueil <paul@crapouillou.net>]

Hi,

Le 16 octobre 2023 01:55:41 GMT+02:00, "Paulo César Pereira de Andrade" <paulo.cesar.pereira.de.andrade@gmail.com> a écrit :
>Em sáb., 14 de out. de 2023 às 04:04, Basile Starynkevitch
><basile@starynkevitch.net> escreveu:
>>
>>
>> On 10/13/23 20:15, Paulo César Pereira de Andrade wrote:
>>
>> Hi Paul. Please sendo the v2 and I will apply the patch during the weekend.
>>
>> Em qua, 11 de out de 2023 12:57, Paul Cercueil <paul@crapouillou.net> escreveu:
>>>
>>> Ooops, I missed to update the source files in check/.
>>>
>>> I'll send a V2.
>>>
>>> -Paul
>>>
>>> Le mercredi 11 octobre 2023 à 17:47 +0200, Paul Cercueil a écrit :
>>> > Allow specifying where Lightning's messages and disassembly will be
>>> > printed, instead of inconditionally using the error output.
>>> > diff --git a/include/lightning.h.in b/include/lightning.h.in
>>> > index 6d51235..25f685b 100644
>>> > --- a/include/lightning.h.in
>>> > +++ b/include/lightning.h.in
>>> > @@ -23,6 +23,7 @@
>>> >  #include <unistd.h>
>>> >  #include <stdlib.h>
>>> >  @MAYBE_INCLUDE_STDINT_H@
>>> > +#include <stdio.h>
>>> >  #include <string.h>
>>> >  #include <pthread.h>
>>> >
>>> > @@ -1220,7 +1221,7 @@ typedef void
>>> > (*jit_free_func_ptr)        (void*);
>>> >  /*
>>> >   * Prototypes
>>> >   */
>>> > -extern void init_jit(const char*);
>>> > +extern void init_jit(const char*,FILE*);
>>> >  extern void finish_jit(void);
>>> >
>>
>>
>> This patch is interesting and provides an interesting feature. However, it is changing the public API of GNU lightning in an incompatible way.
>
>  I did not like it much when first looking at it due to the clear abi break.
>  But the feature is very useful. It just sets the FILE* where binutils is
>told to print the disassembly, as well as where debug printed by Lightning
>is also output.

I can make init_jit() a macro that would call a init_jit_dbg() function (the current init_jit() renamed) with stderr as parameter.

Cheers,
-Paul

>> For projects (like the RefPerSys open source inference engine on https://github.com/RefPerSys/RefPerSys/ ....) which are using GNU lightning it would be very convenient to have a preprocessor convention (inspired by libgccjit on https://gcc.gnu.org/onlinedocs/jit/ ...) which enables users of GNU lightning to test it.
>>
>>
>> What about adding in include/lightning.h.in some preprocessor symbols like
>>
>> #define LIB_LIGHTNING_API 1
>>
>> and increment that number every time the external API of GNU lightning is changing?
>>
>> This would make using GNU lightning easier.
>
>  I believe this is good, but should be avoided and only done when
>really required. For now the proper approach should be to make a
>new function call, to set the disasm_stream pointer in jit_disasm.c.
>
>> Thanks
>>
>> --
>> Basile Starynkevitch
>>  <basile@starynkevitch.net>
>> (only mine opinions / les opinions sont miennes uniquement)
>> 92340 Bourg-la-Reine, France
>> web page: starynkevitch.net/Basile/
>
>Thanks,
>Paulo