[Mesa-dev] Moving amdgpu/addrlib into a git submodule

[Nicolai Hähnle]

On 09.08.2016 19:18, Marek Olšák wrote:
> On Tue, Aug 9, 2016 at 5:35 PM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
>> On 09.08.2016 17:21, Marek Olšák wrote:
>>>
>>> On Tue, Aug 9, 2016 at 3:47 PM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
>>>>
>>>> Hi everybody,
>>>>
>>>> addrlib is the addressing and alignment calculator which is used by
>>>> radeonsi. It's developed (and also used) internally at AMD, and so far
>>>> we've
>>>> had one open source copy living in the Mesa repository at
>>>> src/gallium/winsys/amdgpu/drm/addrlib.
>>>>
>>>> The question of using addrlib in non-Mesa parts of our open-source stack
>>>> has
>>>> come up, in particular in relation to compute. We'd obviously like to
>>>> share
>>>> the code rather than having multiple copies flying around. Since the
>>>> interface of addrlib is slow-moving but unstable, shared linking is not
>>>> an
>>>> option.
>>>>
>>>> I think the best way forward is to create a dedicated repository for
>>>> addrlib
>>>> which is then integrated into Mesa as a git submodule.
>>>>
>>>> The point of this email is to gather feedback from the Mesa community on
>>>> this plan, which is explicitly:
>>>>
>>>> (0) Create an addrlib repository, say amd/addrlib on fd.o.
>>>> (1) Add it as a git submodule to the Mesa repository.
>>>> (2) Fix up whatever aspects of the build system that may be affected
>>>> (perhaps for building source tarballs).
>>>> (3) Go back to mostly ignoring addrlib, except for trying to get better
>>>> at
>>>> syncing with the internal closed-source version.
>>>>
>>>> From initial experiments, the impact on users interested in radeon is
>>>> that
>>>> they will have to run `git submodule init` and then occasionally `git
>>>> submodule update`. Users who do not build radeonsi should be able to
>>>> ignore
>>>> the change completely.
>>>>
>>>> There are alternatives. For example, ROCm uses Google's repo tool
>>>> already.
>>>> But for Mesa, git submodule looks like a lightweight, well supported and
>>>> overall conservative option that everybody should already have installed.
>>>> If
>>>> there are good arguments for something else, let's hear them!
>>>>
>>>> Another point: if we proceed with this plan, I think we should consider
>>>> moving addrlib into src/amd/addrlib. There are two reasons: First,
>>>> transitioning to a submodule *without* changing the directory is probably
>>>> more fragile, i.e. what happens when you switch between checkouts before
>>>> and
>>>> after the transition. Second, if/when radv ends up being merged into Mesa
>>>> master, it makes sense to have addrlib there anyway.
>>>>
>>>> Thoughts? Complaints? Praise?
>>>
>>>
>>> I don't know.
>>>
>>> How does this ensure that Mesa and ROCm addrlib copies won't diverge?
>>
>>
>> They won't really be different copies, because both "copies" are really
>> checkouts from the same repository. They will occasionally be checkouts of
>> _different versions_ from the same repository -- usually that would happen
>> after a sync with the internal copy, when one driver updates their pointer
>> before the other does. But that's easiy to reconcile. Usually it should just
>> mean changing the version pointer in whichever driver uses the older
>> version.
>>
>>
>>> What issues can we expect if Mesa and ROCm addrlib copies diverge?
>>
>>
>> This is about software maintenance. If we _do_ have separate copies, and
>> someone applies a bug fix in one copy, they may forget to apply it to the
>> other. When we want to sync with the internal copy, that has to be done
>> twice. Basically, all the usual frictions that go with having the same (or
>> almost the same) piece of code in more than one place.
>
> Instead of introducing a new repo, can ROCm simply copy addrlib
> directly from the Mesa tree?
>
> If I understand it correctly, the only thing the git submodule would
> allow is that ROCm developers wouldn't have to clone Mesa.

They certainly can copy addrlib from the Mesa tree. The question is 
whether that's a good basis for staying synchronized in the future.

Nicolai

>
> Marek
>