Re: [PATCH] libgccjit Fix a RTL bug for libgccjit

[Jeff Law <jeffreyalaw@gmail.com>]

On 11/20/23 16:54, David Malcolm wrote:
> On Mon, 2023-11-20 at 16:38 -0700, Jeff Law wrote:
>>
>>
>> On 11/20/23 15:46, David Malcolm wrote:
>>> On Fri, 2023-11-17 at 14:09 -0700, Jeff Law wrote:
>>>>
>>>>
>>>> On 11/17/23 14:08, Antoni Boucher wrote:
>>>>> In contrast with the other frontends, libgccjit can be executed
>>>>> multiple times in a row in the same process.
>>>> Yup.  I'm aware of that.  Even so calling init_emit_once more
>>>> than
>>>> one
>>>> time still seems wrong.
>>>
>>> There are two approaches we follow when dealing with state stored
>>> in
>>> global variables:
>>> (a) clean it all up via the various functions called from
>>> toplev::finalize
>>> (b) make it effectively constant once initialized, with idempotent
>>> initialization
>>>
>>> The multiple in-process executions of libgccjit could pass in
>>> different
>>> code-generation options.  Does the RTL-initialization logic depend
>>> anywhere on flags passed in, because if so, we're probably going to
>>> need to re-run the initialization.
>> The INIT_EXPANDERS code would be the most concerning as it's
>> implementation is totally hidden and provided by the target. I
>> wouldn't
>> be at all surprised if one or more do something evil in there.  That
>> probably needs to be evaluated on a target by target basis.
>>
>> The rest really do look like single init, even in a JIT environment
>> kinds of things -- ie all the shared constants in RTL.
> 
> I think Antoni's patch can we described as implementing "single init",
> in that it ensures that at least part of init_emit_once is single init.
> 
> Is the posted patch OK by you, or do we need to rework things, and if
> the latter, what would be the goal?
What I'm struggling with is perhaps a problem of naming.  Conceptually 
"init_emit_once" in my mind should be called once and only once.    If I 
read Antoni's change correctly, we call it more than once.  That just 
feels conceptually wrong -- add to it the opaqueness of INIT_EXPANDERS 
and it feels even more wrong -- we don't know what's going on behind the 
scenes in there.

jeff