Mon Jul 25 18:17:35 PDT 2011

Helo Chandler.

Chandler Carruth <chandlerc at gmail.com>
writes:

> Author: chandlerc
> Date: Mon Jul 25 19:09:08 2011
> New Revision: 136023
>
> URL: http://llvm.org/viewvc/llvm-project?rev=136023&view=rev
> Log:
> Clean up a pile of hacks in our CMake build relating to TableGen.

Thanks for working on this. Tablegenning is a messy area (and you didn't
entered the clang part :-)

> The first problem to fix is to stop creating synthetic *Table_gen
> targets next to all of the LLVM libraries. These had no real effect as
> CMake specifies that add_custom_command(OUTPUT ...) directives (what the
> 'tablegen(...)' stuff expands to) are implicitly added as dependencies
> to all the rules in that CMakeLists.txt.

At the beginning this wasn't true (either a bug or add_custom_command
had not that effect at the time) so the custom targets were
necessary. I'm glad they are gone.

[snip]

> This patch removes these synthetic rules completely, and adds a much
> simpler function to declare explicitly that a collection of tablegen'ed
> files are referenced by other libraries. From that, we can add explicit
> dependencies from the smaller libraries (such as every architectures
> Desc library) on this and correctly form a linear sequence. All of the
> backends are updated to use it, sometimes replacing the existing attempt
> at adding a dependency, sometimes adding a previously missing dependency
> edge.

I'm not sure I get this. Previous to your change, the LLVM library
(LLVMX86CodeGen, for instance) had LLVMX86CodeGen_TableGen as a
dependency (plus the tablegen source files) and its sublibraries
(LLVMX86AsmPrinter, ...) had LLVMX86CodeGen as a dependency. Somehow
this didn't work. So why do we need add_public_tablegen_target at all?
The sublibraries, by depending on LLVMX86CodeGen, should not start
building until after LLVMX86CodeGen is created, which means that the
tablegenning is finished too. The fact that TABLEGEN_OUTPUT keeps its
value when the sublibraries are processed may be a problem, but that's
is trivially solvable (just unset it at the end of add_llvm_target.)

I think that the really nasty problem with tablegenning lies on the
dependencies on .td files included by the main file listed in
LLVM_TARGET_DEFINITIONS. Specifically, on those .td files living outside
CMAKE_CURRENT_SOURCE_DIR. I think that sometimes modifications to those
files are not detected, apart from using a brute-force approach on the
`tablegen' macro for adding them as dependencies to the custom command.
I tried to devise a method for accurately retrieving the dependencies
from the main .td file (X86.td, for instance) but all the possibilities
were too complex.

[snip]

> Modified: llvm/trunk/cmake/modules/TableGen.cmake
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/cmake/modules/TableGen.cmake?rev=136023&r1=136022&r2=136023&view=diff
> ==============================================================================
> --- llvm/trunk/cmake/modules/TableGen.cmake (original)
> +++ llvm/trunk/cmake/modules/TableGen.cmake Mon Jul 25 19:09:08 2011
> @@ -45,23 +45,11 @@
>      PROPERTIES GENERATED 1)
>  endmacro(tablegen)
>  
> -
> -function(create_tablegenning_custom_target target)
> -  # Creates the global target that runs the file-level dependencies
> -  # for tablegenning.
> +function(add_public_tablegen_target target)
> +  # Creates a target for publicly exporting tablegen dependencies.
>    if( TABLEGEN_OUTPUT )
> -    add_custom_target(${target}Table_gen
> +    add_custom_target(${target}
>        DEPENDS ${TABLEGEN_OUTPUT})
> -    add_dependencies(${target}Table_gen ${LLVM_COMMON_DEPENDS})
> +    add_dependencies(${target} ${LLVM_COMMON_DEPENDS})
>    endif( TABLEGEN_OUTPUT )
>  endfunction()
> -
> -function(add_tablegenning_dependency target)
> -  # Makes the tablegenning step created with
> -  # create_tablegenning_custom_target dependent on `target'.
> -  if ( TABLEGEN_OUTPUT )
> -    add_dependencies(${target} ${target}Table_gen)
> -    set_target_properties(${target}Table_gen PROPERTIES FOLDER "Tablegenning")

It would be a good thing if ${target}Table_gen on
add_public_tablegen_target is keep in the "Tablegenning" folder, as
above.

[snip]

> Modified: llvm/trunk/lib/Target/ARM/CMakeLists.txt
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/CMakeLists.txt?rev=136023&r1=136022&r2=136023&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/CMakeLists.txt (original)
> +++ llvm/trunk/lib/Target/ARM/CMakeLists.txt Mon Jul 25 19:09:08 2011
> @@ -13,6 +13,7 @@
>  tablegen(ARMGenSubtargetInfo.inc -gen-subtarget)
>  tablegen(ARMGenEDInfo.inc -gen-enhanced-disassembly-info)
>  tablegen(ARMGenDecoderTables.inc -gen-arm-decoder)
> +add_public_tablegen_target(ARMCommonTableGen)
>  
>  add_llvm_target(ARMCodeGen
>    ARMAsmPrinter.cpp
>
> Modified: llvm/trunk/lib/Target/ARM/Disassembler/CMakeLists.txt
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/Disassembler/CMakeLists.txt?rev=136023&r1=136022&r2=136023&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/Disassembler/CMakeLists.txt (original)
> +++ llvm/trunk/lib/Target/ARM/Disassembler/CMakeLists.txt Mon Jul 25 19:09:08 2011
> @@ -11,4 +11,4 @@
>    PROPERTY COMPILE_FLAGS "/Od"
>    )
>  endif()
> -add_dependencies(LLVMARMDisassembler ARMCodeGenTable_gen)
> +add_dependencies(LLVMARMDisassembler ARMCommonTableGen)

There is a pattern here. add_public_tablegen_target should be called
from add_llvm_target and the add_dependencies for the sublibrary should
be in add_library, the same way we create the dependency on the LLVM
library target:

  if( CURRENT_LLVM_TARGET )
    add_dependencies(${name} ${CURRENT_LLVM_TARGET})
  endif()

[snip]