diff mbox

[1/n] OpenMP 4.0 offloading infrastructure

Message ID 87wq5x5lp7.fsf@kepler.schwinge.homeip.net
State New
Headers show

Commit Message

Thomas Schwinge Dec. 12, 2014, 8:14 a.m. UTC 
Hi!

On Tue, 30 Sep 2014 13:16:37 +0200, I wrote:
> On Fri, 26 Sep 2014 16:36:21 +0400, Ilya Verbin <iverbin@gmail.com> wrote:
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -286,6 +286,24 @@ case ${with_newlib} in
> >    yes) skipdirs=`echo " ${skipdirs} " | sed -e 's/ target-newlib / /'` ;;
> >  esac
> >  
> > +AC_ARG_ENABLE(as-accelerator-for,
> > +[AS_HELP_STRING([--enable-as-accelerator-for=ARG],
> > +		[build as offload target compiler.
> > +		Specify offload host triple by ARG])],
> > +ENABLE_AS_ACCELERATOR_FOR=$enableval,
> > +ENABLE_AS_ACCELERATOR_FOR=no)
> 
> I don't see $ENABLE_AS_ACCELERATOR_FOR being used anywhere, so this can
> probably be removed?

On Wed, 1 Oct 2014 20:05:45 +0400, Ilya Verbin <iverbin@gmail.com> wrote:
> It will be used in one of the upcoming patches.

OK, but why do you need the all-uppercase variant?  The lowercase
enable_as_accelerator_for already is (automatically) populated by
Autoconf, and used in other places?  Here is a untested cleanup patch;
could you please test this?

	* configure.ac (--enable-as-accelerator-for): Don't set
	ENABLE_AS_ACCELERATOR_FOR.  Update all users.
	* configure: Regenerate.



Grüße,
 Thomas

Comments

Jakub Jelinek Dec. 12, 2014, 8:17 a.m. UTC | #1 
On Fri, Dec 12, 2014 at 09:14:28AM +0100, Thomas Schwinge wrote:
> On Tue, 30 Sep 2014 13:16:37 +0200, I wrote:
> > On Fri, 26 Sep 2014 16:36:21 +0400, Ilya Verbin <iverbin@gmail.com> wrote:
> > > --- a/configure.ac
> > > +++ b/configure.ac
> > > @@ -286,6 +286,24 @@ case ${with_newlib} in
> > >    yes) skipdirs=`echo " ${skipdirs} " | sed -e 's/ target-newlib / /'` ;;
> > >  esac
> > >  
> > > +AC_ARG_ENABLE(as-accelerator-for,
> > > +[AS_HELP_STRING([--enable-as-accelerator-for=ARG],
> > > +		[build as offload target compiler.
> > > +		Specify offload host triple by ARG])],
> > > +ENABLE_AS_ACCELERATOR_FOR=$enableval,
> > > +ENABLE_AS_ACCELERATOR_FOR=no)
> > 
> > I don't see $ENABLE_AS_ACCELERATOR_FOR being used anywhere, so this can
> > probably be removed?
> 
> On Wed, 1 Oct 2014 20:05:45 +0400, Ilya Verbin <iverbin@gmail.com> wrote:
> > It will be used in one of the upcoming patches.
> 
> OK, but why do you need the all-uppercase variant?  The lowercase
> enable_as_accelerator_for already is (automatically) populated by
> Autoconf, and used in other places?  Here is a untested cleanup patch;
> could you please test this?
> 
> 	* configure.ac (--enable-as-accelerator-for): Don't set
> 	ENABLE_AS_ACCELERATOR_FOR.  Update all users.
> 	* configure: Regenerate.

Ok if it works.

	Jakub
Kirill Yukhin Dec. 15, 2014, 9:04 a.m. UTC | #2 
Hi,
On 12 Dec 09:17, Jakub Jelinek wrote:
> On Fri, Dec 12, 2014 at 09:14:28AM +0100, Thomas Schwinge wrote:
> > On Tue, 30 Sep 2014 13:16:37 +0200, I wrote:
> > > On Fri, 26 Sep 2014 16:36:21 +0400, Ilya Verbin <iverbin@gmail.com> wrote:
> > > > --- a/configure.ac
> > > > +++ b/configure.ac
> > > > @@ -286,6 +286,24 @@ case ${with_newlib} in
> > > >    yes) skipdirs=`echo " ${skipdirs} " | sed -e 's/ target-newlib / /'` ;;
> > > >  esac
> > > >  
> > > > +AC_ARG_ENABLE(as-accelerator-for,
> > > > +[AS_HELP_STRING([--enable-as-accelerator-for=ARG],
> > > > +		[build as offload target compiler.
> > > > +		Specify offload host triple by ARG])],
> > > > +ENABLE_AS_ACCELERATOR_FOR=$enableval,
> > > > +ENABLE_AS_ACCELERATOR_FOR=no)
> > > 
> > > I don't see $ENABLE_AS_ACCELERATOR_FOR being used anywhere, so this can
> > > probably be removed?
> > 
> > On Wed, 1 Oct 2014 20:05:45 +0400, Ilya Verbin <iverbin@gmail.com> wrote:
> > > It will be used in one of the upcoming patches.
> > 
> > OK, but why do you need the all-uppercase variant?  The lowercase
> > enable_as_accelerator_for already is (automatically) populated by
> > Autoconf, and used in other places?  Here is a untested cleanup patch;
> > could you please test this?
> > 
> > 	* configure.ac (--enable-as-accelerator-for): Don't set
> > 	ENABLE_AS_ACCELERATOR_FOR.  Update all users.
> > 	* configure: Regenerate.
> 
> Ok if it works.
The patch doesn't regress MIC offloading.

--
Thanks, K

> 
> 	Jakub
diff mbox

Patch

diff --git configure configure
index 297f38e..1804198 100755
--- configure
+++ configure
@@ -2893,9 +2893,7 @@  esac
 
 # Check whether --enable-as-accelerator-for was given.
 if test "${enable_as_accelerator_for+set}" = set; then :
-  enableval=$enable_as_accelerator_for; ENABLE_AS_ACCELERATOR_FOR=$enableval
-else
-  ENABLE_AS_ACCELERATOR_FOR=no
+  enableval=$enable_as_accelerator_for;
 fi
 
 
@@ -3094,7 +3092,7 @@  if test "${enable_liboffloadmic+set}" = set; then :
     as_fn_error "--enable-liboffloadmic=no/host/target" "$LINENO" 5 ;;
 esac
 else
-  if test "${ENABLE_AS_ACCELERATOR_FOR}" != "no"; then
+  if test x"$enable_as_accelerator_for" != x; then
   case "${target}" in
     *-intelmic-* | *-intelmicemul-*)
       enable_liboffloadmic=target
diff --git configure.ac configure.ac
index fd1bdf0..91c9a72 100644
--- configure.ac
+++ configure.ac
@@ -289,9 +289,7 @@  esac
 AC_ARG_ENABLE(as-accelerator-for,
 [AS_HELP_STRING([--enable-as-accelerator-for=ARG],
 		[build as offload target compiler.
-		Specify offload host triple by ARG])],
-ENABLE_AS_ACCELERATOR_FOR=$enableval,
-ENABLE_AS_ACCELERATOR_FOR=no)
+		Specify offload host triple by ARG])])
 
 AC_ARG_ENABLE(offload-targets,
 [AS_HELP_STRING([--enable-offload-targets=LIST],
@@ -470,7 +468,7 @@  AC_HELP_STRING([[--enable-liboffloadmic[=ARG]]],
   *)
     AC_MSG_ERROR([--enable-liboffloadmic=no/host/target]) ;;
 esac],
-[if test "${ENABLE_AS_ACCELERATOR_FOR}" != "no"; then
+[if test x"$enable_as_accelerator_for" != x; then
   case "${target}" in
     *-intelmic-* | *-intelmicemul-*)
       enable_liboffloadmic=target