[Hexagon] Select qaic executable based on Ubuntu version

[kparzysz-quic]

Allow users to override the selection via QAIC_PATH_OVERRIDE environment variable, for example on non-Ubuntu systems that can still run Ubuntu binaries.

[kparzysz-quic]

cc: @mehrdadh @supersat

[mehrdadh]

LGTM, thanks!

[cconvey]

Would it be more idiomatic for QAIC_PATH_OVERRIDE to be a CMake variable, rather than an environment variable?

[kparzysz-quic]

The motivation for an environment variable is that one can set it and then use cmake as if nothing happened, so to speak. Since cmake calls itself recursively when building various projects, passing of a cmake variable would need to be explicit.

[cconvey]

Thanks, that makes sense.

[cconvey]

Is there a particular reason to use SEND_ERROR instead of FATAL_ERROR?

[kparzysz-quic]

Changed it to a WARNING---the message is there to inform the user about the override flag.

[cconvey]

Thanks @kparzysz-quic ! There might be one last issue, but that's probably all:

Now when get_hexagon_sdk_property can't obtain a valid path for QAIC_EXE, some (all?) callers need to check for that condition.

It looks like that's already happening in some places, for example:

tvm/src/runtime/hexagon/android/target/fastrpc/CMakeLists.txt

Lines 29 to 39 in 02b2469

	get_hexagon_sdk_property("${HEXAGON_SDK_ROOT}" "${HEXAGON_ARCH}"
	SDK_INCLUDE SDK_INCLUDE_DIRS
	QURT_INCLUDE QURT_INCLUDE_DIRS
	DSPRPC_LIB DSPRPC_LIB_DIRS
	RPCMEM_ROOT RPCMEM_ROOT_DIR
	QAIC_EXE QAIC_EXE_PATH
	)
	if(NOT SDK_INCLUDE_DIRS OR NOT QURT_INCLUDE_DIRS OR NOT DSPRPC_LIB_DIRS OR
	NOT RPCMEM_ROOT_DIR OR NOT QAIC_EXE_PATH)
	message(WARNING "Could not locate some Hexagon SDK components")
	endif()

But not everywhere that (I think) relies on it being valid:

• tvm/cmake/modules/Hexagon.cmake

  Lines 214 to 228 in 02b2469

  	get_hexagon_sdk_property("${USE_HEXAGON_SDK}" "${USE_HEXAGON_ARCH}"
  	SDK_INCLUDE SDK_INCLUDE_DIRS
  	QAIC_EXE QAIC_EXE_PATH
  	)
  	foreach(INCDIR IN LISTS SDK_INCLUDE_DIRS)
  	list(APPEND QAIC_FLAGS "-I${INCDIR}")
  	endforeach()
  	add_custom_command(
  	OUTPUT
  	"${TVMRT_SOURCE_DIR}/hexagon/rpc/hexagon_rpc.h"
  	"${TVMRT_SOURCE_DIR}/hexagon/rpc/hexagon_rpc_skel.c"
  	"${TVMRT_SOURCE_DIR}/hexagon/rpc/hexagon_rpc_stub.c"
  	COMMAND
  	${QAIC_EXE_PATH} ${QAIC_FLAGS}

• tvm/apps/hexagon_launcher/cmake/android/CMakeLists.txt

  Lines 33 to 37 in 02b2469

  	add_custom_command(
  	OUTPUT ${LAUNCHER_RPC_STUB_C} ${LAUNCHER_RPC_H}
  	COMMAND ${QAIC_EXE_PATH} ${QAIC_FLAGS} "${LAUNCHER_SRC}/${LAUNCHER_RPC_IDL}"
  	MAIN_DEPENDENCY "${LAUNCHER_SRC}/${LAUNCHER_RPC_IDL}"
  	)

• tvm/apps/hexagon_launcher/cmake/hexagon/CMakeLists.txt

  Lines 33 to 37 in 02b2469

  	add_custom_command(
  	OUTPUT ${LAUNCHER_RPC_SKEL_C} ${LAUNCHER_RPC_H}
  	COMMAND ${QAIC_EXE_PATH} ${QAIC_FLAGS} "${LAUNCHER_SRC}/${LAUNCHER_RPC_IDL}"
  	MAIN_DEPENDENCY "${LAUNCHER_SRC}/${LAUNCHER_RPC_IDL}"
  	)

[kparzysz-quic]

Now when get_hexagon_sdk_property can't obtain a valid path for QAIC_EXE, some (all?) callers need to check for that condition.

All these checks do is display a warning. In case of a missing qaic, the warning is already shown in the property lookup itself.

[cconvey]

Now when get_hexagon_sdk_property can't obtain a valid path for QAIC_EXE, some (all?) callers need to check for that condition.

All these checks do is display a warning. In case of a missing qaic, the warning is already shown in the property lookup itself.

Right. An earlier version of this PR would cause cmake to fail if the qaic executable couldn't be found.

The current version of this PR would only issue a warning during cmake execution, leaving the hard failure until the make invocation.

I've been assuming that when possible, we want to trap errors during cmake execution rather than waiting until make, ninja, etc. attempt to build those targets.

I'm not aware of TVM having a guideline about this, so if the other reviewers / committers are okay with it, that's fine.