[Mesa-dev] [RFC PATCH] automake: add support to src/glsl/

[Matt Turner]

On Mon, Sep 26, 2011 at 11:29 AM, Eric Anholt <eric at anholt.net> wrote:
> On Sun, 25 Sep 2011 15:36:02 -0400, Matt Turner <mattst88 at gmail.com> wrote:
> Non-text part: multipart/mixed
>> On Sat, Sep 24, 2011 at 9:06 PM, Matt Turner <mattst88 at gmail.com> wrote:
>> > Signed-off-by: Matt Turner <mattst88 at gmail.com>
>> > ---
>> > The last discussion about using automake ("[RFC] Convert mesa to automake/libtool")
>> > ended without anything happening, probably because the branch wasn't ready.
>> >
>> > This patch is an attempt to get the ball rolling again. Without ripping out
>> > the entire existing build system in one swat, it attempts to gradually replace
>> > it directory by directory with automake.
>> >
>> > The patch has a few problems currently, and a few things that can possibly be
>> > done better:
>> >        - Mainly, that building libmesa.a currently fails.
>> >        - Not sure how to handle shared/static dricore options.
>> >        - libtool defines VERSION (-DVERSION=...), which screws up the VERSION
>> >          token in glsl_lexer.ll and glsl_parser.yy. I trivially renamed it.
>> >        - libralloc.la can probably be combined into libglslcore.la, and not
>> >          have to be added to every _LDADD line.
>> >        - The rules for flex and bison can probably be eliminated by using
>> >          YFLAGS and LFLAGS. I tried, but ylwrap gave me some error.
>
> I'd like to add other libs (hash table) at the src/ level, too, so a
> single helper lib that is "mesa's shared, non-mtypes-using stuff but not
> things that are really Mesa like the glsl compiler" would be nice.
>
> I do want to see dricore continue.  It's a really nice feature for
> development, by shortening build times, and it seems like distros want
> it.  However, it was pretty broken as-is.  You had to build every helper
> lib into a separate installed .so, because the build system was lame,
> and dependencies were even more screwed up than usual.  I'd say go ahead
> even if dricore isn't fixed.  I don't think we'll have any big problem
> recovering it for automake once core Mesa is converted.

I definitely want dricore as well.

If you've got a few minutes, it'd be very helpful if you could try the
patch and see if you can see why libmesa.a fails to compile with it.

>> diff --git a/src/glsl/Makefile.am b/src/glsl/Makefile.am
>> new file mode 100644
>> index 0000000..a31d845
>> --- /dev/null
>> +++ b/src/glsl/Makefile.am
>> @@ -0,0 +1,152 @@
>> +LEX = @FLEX@
>> +YACC = @BISON@
>> +
>> +noinst_LTLIBRARIES = \
>> +     libhash_table.la \
>> +     libstandalone_scaffolding.la \
>> +     libsymbol_table.la \
>> +     libmain.la \
>> +     libglcpp.la \
>> +     libglslcore.la
>> +lib_LTLIBRARIES = libglsl.la
>> +noinst_PROGRAMS = glcpp/glcpp builtin_compiler glsl_test
>> +bin_PROGRAMS = glsl_compiler
>> +
>> +INCLUDES = \
>> +        -I. \
>> +     -I../mesa \
>> +     -I../mapi \
>> +     -Iglcpp \
>> +     -I../../include
>> +
>> +BUILT_SOURCES = \
>> +     glcpp/glcpp-lex.c \
>> +     glcpp/glcpp-parse.c \
>> +     glsl_lexer.cpp \
>> +     glsl_parser.cpp \
>> +     builtin_function.cpp
>> +
>> +libhash_table_la_SOURCES = ../mesa/program/hash_table.c
>> +libstandalone_scaffolding_la_SOURCES = standalone_scaffolding.cpp
>> +libsymbol_table_la_SOURCES = ../mesa/program/symbol_table.c
>> +libmain_la_SOURCES = main.cpp
>> +
>> +libglcpp_la_SOURCES = \
>> +     glcpp/glcpp-lex.c \
>> +     glcpp/glcpp-parse.c \
>> +     glcpp/pp.c \
>> +     ralloc.c
>> +
>> +glcpp_glcpp_SOURCES = glcpp/glcpp.c
>> +glcpp_glcpp_LDADD = libglcpp.la libhash_table.la
>> +
>> +libglslcore_la_SOURCES = \
>> +     strtod.c \
>> +     ast_expr.cpp \
>> +     ast_function.cpp \
>> +     ast_to_hir.cpp \
>> +     ast_type.cpp \
>> +     glsl_lexer.cpp \
>> +     glsl_parser.cpp \
>> +     glsl_parser_extras.cpp \
>> +     glsl_types.cpp \
>> +     glsl_symbol_table.cpp \
>> +     hir_field_selection.cpp \
>> +     ir_basic_block.cpp \
>> +     ir_clone.cpp \
>> +     ir_constant_expression.cpp \
>> +     ir.cpp \
>> +     ir_expression_flattening.cpp \
>> +     ir_function_can_inline.cpp \
>> +     ir_function_detect_recursion.cpp \
>> +     ir_function.cpp \
>> +     ir_hierarchical_visitor.cpp \
>> +     ir_hv_accept.cpp \
>> +     ir_import_prototypes.cpp \
>> +     ir_print_visitor.cpp \
>> +     ir_reader.cpp \
>> +     ir_rvalue_visitor.cpp \
>> +     ir_set_program_inouts.cpp \
>> +     ir_validate.cpp \
>> +     ir_variable.cpp \
>> +     ir_variable_refcount.cpp \
>> +     linker.cpp \
>> +     link_functions.cpp \
>> +     loop_analysis.cpp \
>> +     loop_controls.cpp \
>> +     loop_unroll.cpp \
>> +     lower_clip_distance.cpp \
>> +     lower_discard.cpp \
>> +     lower_if_to_cond_assign.cpp \
>> +     lower_instructions.cpp \
>> +     lower_jumps.cpp \
>> +     lower_mat_op_to_vec.cpp \
>> +     lower_noise.cpp \
>> +     lower_texture_projection.cpp \
>> +     lower_variable_index_to_cond_assign.cpp \
>> +     lower_vec_index_to_cond_assign.cpp \
>> +     lower_vec_index_to_swizzle.cpp \
>> +     lower_vector.cpp \
>> +     opt_algebraic.cpp \
>> +     opt_constant_folding.cpp \
>> +     opt_constant_propagation.cpp \
>> +     opt_constant_variable.cpp \
>> +     opt_copy_propagation.cpp \
>> +     opt_copy_propagation_elements.cpp \
>> +     opt_dead_code.cpp \
>> +     opt_dead_code_local.cpp \
>> +     opt_dead_functions.cpp \
>> +     opt_discard_simplification.cpp \
>> +     opt_function_inlining.cpp \
>> +     opt_if_simplification.cpp \
>> +     opt_noop_swizzle.cpp \
>> +     opt_redundant_jumps.cpp \
>> +     opt_structure_splitting.cpp \
>> +     opt_swizzle_swizzle.cpp \
>> +     opt_tree_grafting.cpp \
>> +     s_expression.cpp
>> +libglslcore_la_LIBADD = libglcpp.la
>> +
>> +builtin_compiler_SOURCES = builtin_stubs.cpp
>> +builtin_compiler_LDADD = \
>> +     libmain.la \
>> +     libglslcore.la \
>> +     libhash_table.la \
>> +     libsymbol_table.la \
>> +     libstandalone_scaffolding.la
>> +
>> +libglsl_la_SOURCES = builtin_function.cpp
>> +libglsl_la_LIBADD = libglslcore.la
>> +
>> +glsl_compiler_SOURCES =
>> +glsl_compiler_LDADD = \
>> +     libmain.la \
>> +     libglsl.la \
>> +     libhash_table.la \
>> +     libsymbol_table.la \
>> +     libstandalone_scaffolding.la
>> +
>> +glsl_test_SOURCES = \
>> +     test.cpp \
>> +     test_optpass.cpp
>> +glsl_test_LDADD = \
>> +     libglsl.la \
>> +     libhash_table.la \
>> +     libsymbol_table.la \
>> +     libstandalone_scaffolding.la
>> +
>> +glsl_lexer.cpp: glsl_lexer.ll
>> +     $(LEX) --nounistd -o$@  $<
>> +
>> +glsl_parser.cpp: glsl_parser.yy
>> +     $(YACC) -v -o "$@" -p "_mesa_glsl_" --defines=glsl_parser.h $<
>> +
>> +glcpp/glcpp-lex.c: glcpp/glcpp-lex.l
>> +     $(LEX) --nounistd -o$@  $<
>> +
>> +glcpp/glcpp-parse.c: glcpp/glcpp-parse.y
>> +     $(YACC) -v -o "$@" --defines=glcpp/glcpp-parse.h $<
>> +
>> +builtin_function.cpp: builtins/profiles/* builtins/ir/* builtins/tools/generate_builtins.py builtins/tools/texture_builtins.py builtin_compiler
>> +     @echo Regenerating builtin_function.cpp...
>> +     $(PYTHON2) $(PYTHON_FLAGS) builtins/tools/generate_builtins.py ./builtin_compiler > builtin_function.cpp || rm -f builtin_function.cpp
>
> I really like how this file is understandable now.  It was a disaster
> before.
>
> One concern I had for automake overall doesn't look like an issue using
> the "include" directive: android mandates using the special android
> build system to build its software, and we're definitely supporting
> android, so we want to share our source lists in a sources.mak file if
> possible.

Definitely. I was doing that when working on other directories, but
src/glsl didn't have a sources.mak for some reason. That's probably a
good first step.

>> diff --git a/src/glsl/glsl_lexer.ll b/src/glsl/glsl_lexer.ll
>> index cfd8926..dd7c68c 100644
>> --- a/src/glsl/glsl_lexer.ll
>> +++ b/src/glsl/glsl_lexer.ll
>> @@ -104,8 +104,8 @@ HASH              ^{SPC}#{SPC}
>>
>>      /* Preprocessor tokens. */
>>  ^[ \t]*#[ \t]*$                      ;
>> -^[ \t]*#[ \t]*version                { BEGIN PP; return VERSION; }
>> -^[ \t]*#[ \t]*extension              { BEGIN PP; return EXTENSION; }
>> +^[ \t]*#[ \t]*version                { BEGIN PP; return GLSL_VERS; }
>> +^[ \t]*#[ \t]*extension              { BEGIN PP; return GLSL_EXTENSION; }
>>  {HASH}line{SPCP}{INT}{SPCP}{INT}{SPC}$ {
>>                                  /* Eat characters until the first digit is
>>                                   * encountered
>> diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
>> index 25d02fb..831f72a 100644
>> --- a/src/glsl/glsl_parser.yy
>> +++ b/src/glsl/glsl_parser.yy
>> @@ -106,7 +106,7 @@
>>  %token INVARIANT
>>  %token LOWP MEDIUMP HIGHP SUPERP PRECISION
>>
>> -%token VERSION EXTENSION LINE COLON EOL INTERFACE OUTPUT
>> +%token GLSL_VERS GLSL_EXTENSION LINE COLON EOL INTERFACE OUTPUT
>>  %token PRAGMA_DEBUG_ON PRAGMA_DEBUG_OFF
>>  %token PRAGMA_OPTIMIZE_ON PRAGMA_OPTIMIZE_OFF
>>  %token PRAGMA_INVARIANT_ALL
>> @@ -228,7 +228,7 @@ translation_unit:
>>
>>  version_statement:
>>       /* blank - no #version specified: defaults are already set */
>> -     | VERSION INTCONSTANT EOL
>> +     | GLSL_VERS INTCONSTANT EOL
>>       {
>>          bool supported = false;
>>
>> @@ -296,7 +296,7 @@ any_identifier:
>>       ;
>>
>>  extension_statement:
>> -     EXTENSION any_identifier COLON any_identifier EOL
>> +     GLSL_EXTENSION any_identifier COLON any_identifier EOL
>>       {
>>          if (!_mesa_glsl_process_extension($2, & @2, $4, & @4, state)) {
>>             YYERROR;
>> --
>> 1.7.3.4
>>
>
> I'd prefer VERSION to be fully spelled out, and it's probably worth
> being in a separate commit.

Yeah, will do. I feel like there's a better way to deal with this anyway.

Thanks,
Matt