This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 3/4] Add libgomp plugin for Intel MIC
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Aleksander Ivanyushenko <aleksander dot ivanyushenko at gmail dot com>
- Cc: kirill dot yukhin at gmail dot com, iverbin at gmail dot com, gcc-patches at gcc dot gnu dot org
- Date: Mon, 30 Nov 2015 16:23:00 +0100
- Subject: Re: [PATCH 3/4] Add libgomp plugin for Intel MIC
- Authentication-results: sourceware.org; auth=none
- References: <20151111145615 dot GA4807 at msticlxl57 dot ims dot intel dot com>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
On Wed, Nov 11, 2015 at 05:56:15PM +0300, Aleksander Ivanyushenko wrote:
> diff --git a/configure.ac b/configure.ac
> index 9241261..b997646 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -494,6 +494,18 @@ else
> fi])
> AC_SUBST(extra_liboffloadmic_configure_flags)
>
> +# Intelmic and intelmicemul require xxd or python.
> +case "${target}" in
> + *-intelmic-* | *-intelmicemul-*)
> + AC_CHECK_PROG(xxd_present, xxd, "yes", "no")
> + AC_CHECK_PROG(python2_present, python2, "yes", "no")
> + AC_CHECK_PROG(python3_present, python3, "yes", "no")
> + if test "$xxd_present$python2_present$python3_present" = "nonono"; then
> + AC_MSG_ERROR([cannot find neither xxd nor python])
> + fi
> + ;;
> +esac
Why here? I'd do something like that only in
liboffloadmic/plugin/configure.ac. Furthermore, it is inconsistent
with what you actually use in liboffloadmic/plugin (where you look only
for python and above you only look for python[23]).
> @@ -73,7 +75,7 @@ main_target_image.h: offload_target_main
> @echo "};" >> $@
> @echo "extern \"C\" const MainTargetImage main_target_image = {" >> $@
> @echo " image_size, \"offload_target_main\"," >> $@
> - @cat $< | xxd -include >> $@
> + @if test "x$(xxd_path)" != "xno"; then cat $< | $(xxd_path) -include >> $@; else $(python_path) $(XXD_PY) $< >> $@; fi;
> @echo "};" >> $@
I'd prefer to use $(XXD) and $(PYTHON) instead of $(xxd_path) and $(python_path),
that is more consistent with dozens of other variables for other tools.
> --- a/liboffloadmic/plugin/configure.ac
> +++ b/liboffloadmic/plugin/configure.ac
> @@ -124,6 +124,10 @@ case ${enable_version_specific_runtime_libs} in
> ;;
> esac
>
> +# Find path to xxd or python
> +AC_PATH_PROG(xxd_path, xxd, "no")
> +AC_PATH_PROG(python_path, python, "no")
I'd use
+AC_PATH_PROG(XXD, xxd, no)
+AC_PATH_PROGS(PYTHON, python python2 python3, no)
and then add the conditional AC_MSG_ERROR if
x$XXD = xno && x$PYTHON = xno
Jakub