Template used to generate isa.h header is outdated

[shramov]

Latest update of specifications/aie2ps/isa.h intoroduced in #134 is not syncrhonized with templates/aie2ps/assembler_stub.h

When build system tries to regenerate header result is equivalent to commit before 3c4d2d8 (normally isa-spec-gen target is not called but Ninja backend thinks that it is needed)

[shramov]

@donwalkarsoham can you take a look?

[lubosz]

I can reproduce this issue.
Every Ninja invocation after the first calls the spec_tool.py and generates a header with the isa_disassembler class missing, which is used in disassembler.h.

Weirdly it works in the first invocation of the build, where the header is not generated.

I can confirm that the specification/aie2ps/isa.h header is not regenerated at all when using the -G "Unix Makefiles" backend of cmake. Also for multiple invocations of the build.

This also breaks the XRT build for me, in case I invoke the build another time after the first.

It seems to be a build system bug that the header is only generated the second invocation of the build.

I suppose generation of this header should be either made mandatory or removed from the build system. In case the header needs to be regenerated on every system, the generation should not overwrite a header in the version controlled source directory but, it should be placed in the temporary build directory as build artifact.

[lubosz]

This issue comes down to my initial suspicion of overwriting source files by build targets.

Ninja invokes the cpp-assembler-stubs phony target due to the definition of BYPRODUCTS in the add_custom_target.

This target is not automatically called for all, as intended, when no file from the source directory is in the BYPRODUCTS.

Commenting the BYPRODUCTS line avoids the issue.

According to the CMake documentation, BYPRODUCTS behave differently for Makefile and Ninja generators:
https://cmake.org/cmake/help/latest/command/add_custom_target.html

Another nasty outcome of defining a version controller source file as BYPRODUCTS is that it is deleted when calling the clean target with the Makefile generator

cmake -G "Unix Makefiles" -B build-make
make -C build-make/ clean

This deletes a bunch of files in the specification directory including the header in question.

Quick fix

Not specifying the version controlled header in question as byproduct avoids the issue by not executing the phony cpp-assembler-stubs target.

diff --git a/specification/aie2ps/CMakeLists.txt b/specification/aie2ps/CMakeLists.txt
index 2e55f65..889474d 100644
--- a/specification/aie2ps/CMakeLists.txt
+++ b/specification/aie2ps/CMakeLists.txt
@@ -8,7 +8,6 @@ add_custom_target(cpp-assembler-stubs
   ${AIEBU_SOURCE_DIR}/specification/aie2ps/isa-spec.yaml
   ${AIEBU_SOURCE_DIR}/templates/aie2ps generate_cpp_assembler_stubs
   > ${AIEBU_SOURCE_DIR}/specification/aie2ps/isa.h
-  BYPRODUCTS ${AIEBU_SOURCE_DIR}/specification/aie2ps/isa.h
   WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
   SOURCES ${AIEBU_SOURCE_DIR}/specification/spec_tool.py
   ${AIEBU_SOURCE_DIR}/specification/aie2ps/isa-spec.yaml

Proper fix

Do not overwrite version controller source files in build targets, but rather call spec_tool.py manually if needed and commit the result.
In case the header needs to be generated every invocation of the build, which was not the indented behavior currently, it should be generated into the build directory.

[shramov]

Template needed to build isa.h header is recreated in #148