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

[Emil Velikov]

On 30 March 2015 at 18:10, Matt Turner <mattst88 at gmail.com> wrote:
> 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.
>
Hmm... ok. Can you elaborate if on the scenario mentioned above "one
may try to "build" nir.h ..." is feasible or a complete BS ? I was
under the impression that make will not wait for all the BUILT_SOURCES
to be generated before starting the actual build. Could be wrong
though :-)

Wrt the author of that hunk I will kindly point out to Connor/Jason -
see commit fa4bc6c1309(nir: use Python to autogenerate opcode
information). They will be able to provide you with additional
information.

Cheers
Emil