[Mesa-dev] [PATCH] nir: acknowledge the existence of nir_builder.h

[Emil Velikov]

On 30 March 2015 at 17:37, Eric Anholt <eric at anholt.net> wrote:
> Emil Velikov <emil.l.velikov at gmail.com> writes:
>
>> On 30/03/15 02:05, Eric Anholt wrote:
>>> Emil Velikov <emil.l.velikov at gmail.com> writes:
>>>
...
>>>> diff --git a/src/glsl/Makefile.am b/src/glsl/Makefile.am
>>>> index ed90366..58af166 100644
>>>> --- a/src/glsl/Makefile.am
>>>> +++ b/src/glsl/Makefile.am
>>>> @@ -244,6 +244,8 @@ nir/nir_builder_opcodes.h: nir/nir_opcodes.py nir/nir_builder_opcodes_h.py
>>>>     $(MKDIR_P) nir;                                                 \
>>>>     $(PYTHON2) $(PYTHON_FLAGS) $(srcdir)/nir/nir_builder_opcodes_h.py > $@
>>>>
>>>> +nir/nir_builder.h: nir/nir_builder_opcodes.h
>>>> +
>>>>  nir/nir_constant_expressions.c: nir/nir_opcodes.py nir/nir_constant_expressions.py nir/nir_constant_expressions.h
>>>>     $(MKDIR_P) nir;                                                 \
>>>>     $(PYTHON2) $(PYTHON_FLAGS) $(srcdir)/nir/nir_constant_expressions.py > $@
>>>
>>> This is weird -- nir_builder.h isn't a build target that needs to be
>>> regenerated.  What's it for?
>>>
>> True, nir_builder.h does not need to be generated, but it depends on a
>> file that is - nir_builder_opcodes.h.  Thus we should note that
>> dependency. Plus it makes sense given a similar rule a few lines below.
>>
>> nir/nir.h: nir/nir_opcodes.h
>
> We don't note dependencies for any other headers that get included by
> things that aren't generated based on the contents of those headers.  So
> I don't get what this dependency is supposed to be doing.

Not sure I get that. Presently in the makefile there is a explicit
dependency link between nir.h and nir_opcodes.h. Imho it  makes
perfect sense, as one may try to "build" nir.h before nir_opcodes.h is
generated. As the former explicitly includes the latter this will
likely lead to compilation failure. With this patch I'm applying the
same rule with a trivial change - nir* becomes nir_builder*.

Does this make more sense or am I missing something ?

-Emil

P.S. Yes, it is a bit hairy situation :-)