[llvm-commits] [llvm] r136023 - in /llvm/trunk: cmake/modules/ lib/Target/ARM/ lib/Target/ARM/AsmParser/ lib/Target/ARM/Disassembler/ lib/Target/ARM/InstPrinter/ lib/Target/ARM/MCTargetDesc/ lib/Target/Alpha/ lib/Target/Alpha/MCTargetDesc/ lib/Target/Alpha/Targ

[Óscar Fuentes]

Chandler Carruth <chandlerc at google.com> writes:

>> > -    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.
>>
>
> I don't know what this does, but I'll try to look into it... Fixing build
> issues is my primary concern.

It puts the target on the correspondingly named folder on some IDEs
(Visual Studio). Otherwise the project view gets cluttered. Just
convenient aesthetics.

>> -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:
>>
>
> No, this is not at all correct. There are only a very very few llvm targets
> which have tablegen files associated with them.

See my previous message for my comments on this.

> Also, fundamentally, this is
> an incorrect model of the dependencies. Both the library and what you are
> (mistakenly) calling a sublibrary depend on the tablegen files. Therefore
> there should be an explicit rule for them both to refer to. For example, we
> might delete the add_llvm_library from the target directory entirely, and
> keep the tablegen and sublibraries.
>
> Similarly, many sublibraries don't need the tablegen from the parent target
> directory, and we shouldn't assume they all do.
>
> My patch moves us toward an explict model of the dependencies in the CMake
> build. That is the correct direction, and the one we are consistently moving
> toward. I even have some hope of getting explicit dependencies in the
> Makefile based build. Dependencies are a contract and a specification, not
> something tools should deduce on the fly.

I don't concur with this. It is very nice to talk about contracts, but
the reality is that build (and library) dependencies are transitive and,
in addition, you usually can reach a given dependecy through more than
one path. Taking time on writing contracts is good if and only if they
are enforced. In practice, you can state a set of build dependencies
that omits certain edges and lists unnecessary edges and the build will
work on some circunstances, and fail on another ones (usually those that
you didn't tested.) That usually is hard to debug, moreover if you
consider that different tools have different heuristics and parallel
builds makes things really nasty.

I've discussed this with other LLVM members on the past. My stance is
that tools are here for helping us and build reliability is a top
requirement. Going to a model that creates more opportunities for
breakage due to forgetting to update or mistakengly updating the
"contracts" is wrong, imho. See how often people forgets to update the
CMakeLists.txt files. Fixing a missing dependecy will not be so easy as
adding or removing a file name from a list.

I'm not saying that I will oppose or reject such changes. I'm just
giving my opinion: explicit dependencies will become outdated and create
build failures, requiring more maintenance work, with no added benefit.