[Mesa-dev] new dispatch generator broke with Marek's parallel compile commit

[Ian Romanick]

On 4/3/19 10:20 AM, Emil Velikov wrote:
> On Tue, 2 Apr 2019 at 20:00, Ian Romanick <idr at freedesktop.org> wrote:
>>
>> On 4/2/19 4:43 AM, Emil Velikov wrote:
>>> On Tue, 2 Apr 2019 at 04:55, Dave Airlie <airlied at gmail.com> wrote:
>>>>
>>>> On Tue, 2 Apr 2019 at 11:24, Dave Airlie <airlied at gmail.com> wrote:
>>>>>
>>>>> Marek's commit to add ARB_parallel_shader_compile broke some es1 tests
>>>>> in the Intel CI.
>>>>>
>>>>> It appears the whatever generates the es1api isn't consistent, for
>>>>> example glTranslatex on my local system is 1405 in es1api but is 1406
>>>>> in the gl api.
>>>>>
>>>>> I'm no expert on this area, Emil any ideas?
>>>>
>>>> This seems to be due the new registry xml parser, I'm not sure how
>>>> broken it is, but it seems like it's a bit busted, and nobody tested
>>>> the scenario where a new function gets introduced in the middle.
>>>>
>>>> It looks like static_data.py has a limit on the offsets it cares
>>>> about, I thought adding static offsets for these functions would help
>>>> here, but it appears currently it all just work by luck, that the
>>>> static offsets work out to be the same as ones generated by gl_XML.py
>>>> for values above MAX_OFFSETS.
>>>>
>>>> I've got a hacky patch that makes it work here, that increases
>>>> MAX_OFFSETS to 1420, adds a new entry to the end for the new APIs, but
>>>> really I think the current code is broken, and is happening to work
>>>> out, but I'm hoping I'm just missing something obvious and it'll be a
>>>> one line fix for Emil.
>>>>
>>> As you have noticed the old generator would add entries to the glapi
>>> table in arbitrary order.
>>> Meaning that the ABI between dri/glapi/libGL* would break every now and then.
>>>
>>> In more detail - libGL* would expect glFooBar at offset X, while the
>>> function is at Y according to glapi and the dri module sets the
>>> dispatch at Y. Latter uses a combination of fixed offset and dynamic
>>> offset lookup.
>>
>> This doesn't make sense to me.  Can you explain more?  There are only
>> two parts.  There's the loader, and there's the driver.  The loader
>> assigns locations either at compile-time or at run-time.  The driver
>> queries the locations of functions that it will provide.  All functions
>> that the loader knows about should realistically have a dispatch
>> location set at compile time.  The loader should only generate new
>> locations at run-time if the application or driver asks for a function
>> that it does not know.
>>
>> For things that don't have a static dispatch location set in the XML,
>> the loader is free to assign any location is pleases, but that location
>> must be consistent across all APIs because a single application can
>> create contexts from every possible API in the same address space, but
>> glXGetProcAddress or eglGetProcAddress are API agnostic.
>>
>> If there is any possibility for a single function to have different
>> dispatch locations in different APIs, then the single most fundamental
>> invariant of the entire dispatch table system is being violated.  That
>> is a pretty catastrophic failure.
>>
> Note: I'm not 100% sure if the following is an issue or design decision.
> I'm inclined towards the former, although I could be wrong.
> 
> Loader references known functions entries by glapi_table offset,
> they're resolved at build-time.
> At some point a developer:
>  - changes the include order in our XMLs
>  - adds a new XML/entrypoints (say glFooBar) at in "special" order
>  - flips the aliasing order of glFoo and glFooARB
> 
> Since our old parser validates the entrypoints in the order they're
> presented**  - glFooBar gets an offset of X and shifts (some of) the
> existing entrypoints by 1.
> Say glHamSandwich - moves from X to X+1
> 
> The driver populates the entrypoints by offset, hence they're resolved
> at build-time.

There are a small number of functions that have locations explicitly set
in our XML.  These are the ones with the static_offset tag.  All of the
other functions must be queried by the driver.  The driver has its own
separate table that it uses to manage the mapping of names to dispatch
offsets.  This is the "remap table."  The remap table has fixed offsets
for each function, and each location in the table stores that location
in the dispatch table of the actual function.  So, to set the dispatch
pointer for glHamSandwich, the driver does something like:

    dispatch_table[remap_table[REMAP_OFFSET_OF_HAMSANDWICH]] =
        _mesa_glHamSandwich;

I don't recall exactly how aliases are handled.  I seem to remember that
there were some issues with alias ordering, so I thought we modified it
so that there was a sort order that preferred lack of suffix, then ARB,
then EXT, then alphabetic.  It has probably been 10 years or more since
I actually worked on that, so my memory may be wrong or it may have
changed... or both.

> New driver sets the glHamSandwich entrypoint at X+1 while old loader
> expect it at X.
> 
> Unless I've missed something, the above will result in sub-par experience.
> 
> HTH
> Emil
> 
> ** The aliasing rules apply on top.
> 
> Aside: while porting libGL and libglapi (WIP) I've noticed two type of
> some aliasing changes wrt upstream.
> I'm planning to add a quirk table and we can go through each one at
> time permits.
> 
> Example:
>  - us AreTexturesResident and AreTexturesResidentEXT are aliased,
> upstream - they're not
>  - us MultiDrawElementsIndirectAMD alias MultiDrawElementsIndirect,
> upstream MultiDrawElementsIndirect alias MultiDrawElementsIndirectAMD
>