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

[Matt Turner]

On Mon, Mar 30, 2015 at 9:58 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> 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 ?

No, it doesn't make sense. Presumably you added it because it fixed a
build error? Maybe if you showed what the error was it would make
sense.