Re: [PATCH 2/4] Add liboffloadmic

2015-07-13 Thread Ilya Verbin 
On Thu, Jul 09, 2015 at 12:00:29 +0200, Thomas Schwinge wrote:
> I noticed that -- at least with current versions of GCC -- there are
> several compiler diagnostics displayed during the build.  It would be
> nice to get these addressed -- as applicable, presumably in the Intel
> upstream version, and then a new import be done into GCC?  For example, I
> noticed the following changes in my build logs (not a complete list):
> 
> {+[...]/source-gcc/liboffloadmic/runtime/emulator/coi_device.cpp:112:28: 
> warning: invalid suffix on literal; C++11 requires a space between literal 
> and string macro [-Wliteral-suffix]+}
> {+   sprintf (pipe_host_path, "%s"PIPE_HOST_PATH, mic_dir);+}
> {+^+}
> {+[...]/source-gcc/liboffloadmic/runtime/emulator/coi_device.cpp:113:30: 
> warning: invalid suffix on literal; C++11 requires a space between literal 
> and string macro [-Wliteral-suffix]+}
> {+   sprintf (pipe_target_path, "%s"PIPE_TARGET_PATH, mic_dir);+}
> {+  ^+}
> 
> {+[...]/source-gcc/liboffloadmic/runtime/emulator/coi_host.cpp:892:24: 
> warning: invalid suffix on literal; C++11 requires a space between literal 
> and string macro [-Wliteral-suffix]+}
> {+   sprintf (pipes_path, "%s"PIPES_PATH, eng->dir);+}
> {+^+}
> {+[...]/source-gcc/liboffloadmic/runtime/emulator/coi_host.cpp:903:28: 
> warning: invalid suffix on literal; C++11 requires a space between literal 
> and string macro [-Wliteral-suffix]+}
> {+   sprintf (pipe_host_path, "%s"PIPE_HOST_PATH, eng->dir);+}
> {+^+}
> {+[...]/source-gcc/liboffloadmic/runtime/emulator/coi_host.cpp:904:30: 
> warning: invalid suffix on literal; C++11 requires a space between literal 
> and string macro [-Wliteral-suffix]+}
> {+   sprintf (pipe_target_path, "%s"PIPE_TARGET_PATH, eng->dir);+}
> {+  ^+}
> 
> [...]/source-gcc/liboffloadmic/runtime/offload_host.cpp:107:30: warning: 
> [-deprecated conversion from-]{+ISO C++ forbids converting a+} string 
> constant to 'char*' [-Wwrite-strings]
>  static char *timer_envname = "H_TIME";
>   ^
> 
> [...]/source-gcc/liboffloadmic/runtime/offload_myo_host.cpp: In function 
> 'void __intel_cilk_for_32_offload(int, void (*)(void*, void*), int, void*, 
> void*, unsigned int, unsigned int)':
> [...]/source-gcc/liboffloadmic/runtime/offload_myo_host.cpp:762:55: 
> warning: [-deprecated conversion from-]{+ISO C++ forbids converting a+} 
> string constant to 'char*' [-Wwrite-strings]
> args, target_number)
>^
> [...]/source-gcc/liboffloadmic/runtime/offload_myo_host.cpp: In function 
> 'void __intel_cilk_for_64_offload(int, void (*)(void*, void*), int, void*, 
> void*, uint64_t, uint64_t)':
> [...]/source-gcc/liboffloadmic/runtime/offload_myo_host.cpp:815:49: 
> warning: [-deprecated conversion from-]{+ISO C++ forbids converting a+} 
> string constant to 'char*' [-Wwrite-strings]
> target_number)
>  ^
> 
> [...]/source-gcc/liboffloadmic/runtime/offload_orsl.cpp:39:33: warning: 
> [-deprecated conversion from-]{+ISO C++ forbids converting a+} string 
> constant to 'ORSLTag {aka char*}' [-Wwrite-strings]
>  static const ORSLTag   my_tag = "Offload";

Yeah, they are already fixed in the upstream version.  I'll prepare an update
for GCC soon.

  -- Ilya

Re: [PATCH 2/4] Add liboffloadmic

2015-07-09 Thread Thomas Schwinge 
Hi Ilya!

On Tue, 21 Oct 2014 21:20:34 +0400, Ilya Verbin  wrote:
> This patch contains liboffloadmic library.
> 
> It is used by ICC for offloading.  The sources are imported from upstream
> ( https://www.openmprtl.org/sites/default/files/liboffload_oss.tgz )
> Configure and makefiles are new.
> 
> Also liboffloadmic/runtime/emulator directory is new.  [...]

I noticed that -- at least with current versions of GCC -- there are
several compiler diagnostics displayed during the build.  It would be
nice to get these addressed -- as applicable, presumably in the Intel
upstream version, and then a new import be done into GCC?  For example, I
noticed the following changes in my build logs (not a complete list):

{+[...]/source-gcc/liboffloadmic/runtime/emulator/coi_device.cpp:112:28: 
warning: invalid suffix on literal; C++11 requires a space between literal and 
string macro [-Wliteral-suffix]+}
{+   sprintf (pipe_host_path, "%s"PIPE_HOST_PATH, mic_dir);+}
{+^+}
{+[...]/source-gcc/liboffloadmic/runtime/emulator/coi_device.cpp:113:30: 
warning: invalid suffix on literal; C++11 requires a space between literal and 
string macro [-Wliteral-suffix]+}
{+   sprintf (pipe_target_path, "%s"PIPE_TARGET_PATH, mic_dir);+}
{+  ^+}

{+[...]/source-gcc/liboffloadmic/runtime/emulator/coi_host.cpp:892:24: 
warning: invalid suffix on literal; C++11 requires a space between literal and 
string macro [-Wliteral-suffix]+}
{+   sprintf (pipes_path, "%s"PIPES_PATH, eng->dir);+}
{+^+}
{+[...]/source-gcc/liboffloadmic/runtime/emulator/coi_host.cpp:903:28: 
warning: invalid suffix on literal; C++11 requires a space between literal and 
string macro [-Wliteral-suffix]+}
{+   sprintf (pipe_host_path, "%s"PIPE_HOST_PATH, eng->dir);+}
{+^+}
{+[...]/source-gcc/liboffloadmic/runtime/emulator/coi_host.cpp:904:30: 
warning: invalid suffix on literal; C++11 requires a space between literal and 
string macro [-Wliteral-suffix]+}
{+   sprintf (pipe_target_path, "%s"PIPE_TARGET_PATH, eng->dir);+}
{+  ^+}

[...]/source-gcc/liboffloadmic/runtime/offload_host.cpp:107:30: warning: 
[-deprecated conversion from-]{+ISO C++ forbids converting a+} string constant 
to 'char*' [-Wwrite-strings]
 static char *timer_envname = "H_TIME";
  ^

[...]/source-gcc/liboffloadmic/runtime/offload_myo_host.cpp: In function 
'void __intel_cilk_for_32_offload(int, void (*)(void*, void*), int, void*, 
void*, unsigned int, unsigned int)':
[...]/source-gcc/liboffloadmic/runtime/offload_myo_host.cpp:762:55: 
warning: [-deprecated conversion from-]{+ISO C++ forbids converting a+} string 
constant to 'char*' [-Wwrite-strings]
args, target_number)
   ^
[...]/source-gcc/liboffloadmic/runtime/offload_myo_host.cpp: In function 
'void __intel_cilk_for_64_offload(int, void (*)(void*, void*), int, void*, 
void*, uint64_t, uint64_t)':
[...]/source-gcc/liboffloadmic/runtime/offload_myo_host.cpp:815:49: 
warning: [-deprecated conversion from-]{+ISO C++ forbids converting a+} string 
constant to 'char*' [-Wwrite-strings]
target_number)
 ^

[...]/source-gcc/liboffloadmic/runtime/offload_orsl.cpp:39:33: warning: 
[-deprecated conversion from-]{+ISO C++ forbids converting a+} string constant 
to 'ORSLTag {aka char*}' [-Wwrite-strings]
 static const ORSLTag   my_tag = "Offload";
 ^


Grüße,
 Thomas


signature.asc
Description: PGP signature

Re: [PATCH 2/4] Add liboffloadmic

2015-02-04 Thread Jakub Jelinek 
On Wed, Feb 04, 2015 at 08:44:42PM +0300, Ilya Verbin wrote:
> contrib/
>   * gcc_update (files_and_dependencies): Add rules for liboffloadmic and
>   liboffloadmic/plugin.

Ok, thanks.

Jakub

Re: [PATCH 2/4] Add liboffloadmic

2015-02-04 Thread Ilya Verbin 
Hi,

On 12 Dec 11:46, Thomas Schwinge wrote:
> On Tue, 21 Oct 2014 21:20:34 +0400, Ilya Verbin  wrote:
> > This patch contains liboffloadmic library.
> 
> > liboffloadmic/
> > Initial commit.  Imported from upstream:
> > https://www.openmprtl.org/sites/default/files/liboffload_oss.tgz
> > * Makefile.am: New file.
> > * Makefile.in: New file, generated by automake.
> > * aclocal.m4: New file, generated by aclocal.
> > * configure: New file, generated by autoconf.
> > * configure.ac: New file.
> 
> contrib/gcc_update:files_and_dependencies needs to be updated for those
> build machinery files as well as those added in liboffloadmic/plugin/
> later on.

This patch adds rules for liboffloadmic and liboffloadmic/plugin to gcc_update.
OK for trunk?


contrib/
* gcc_update (files_and_dependencies): Add rules for liboffloadmic and
liboffloadmic/plugin.


diff --git a/contrib/gcc_update b/contrib/gcc_update
index 5ba3a05..2df9da4 100755
--- a/contrib/gcc_update
+++ b/contrib/gcc_update
@@ -167,6 +167,12 @@ libvtv/configure: libvtv/configure.ac libvtv/aclocal.m4
 libcilkrts/aclocal.m4: libcilkrts/configure.ac
 libcilkrts/Makefile.in: libcilkrts/Makefile.am
 libcilkrts/configure: libcilkrts/configure.ac
+liboffloadmic/aclocal.m4: liboffloadmic/configure.ac
+liboffloadmic/Makefile.in: liboffloadmic/Makefile.am
+liboffloadmic/configure: liboffloadmic/configure.ac
+liboffloadmic/plugin/aclocal.m4: liboffloadmic/plugin/configure.ac
+liboffloadmic/plugin/Makefile.in: liboffloadmic/plugin/Makefile.am
+liboffloadmic/plugin/configure: liboffloadmic/plugin/configure.ac
 # Top level
 Makefile.in: Makefile.tpl Makefile.def
 configure: configure.ac config/acx.m4


  -- Ilya

Re: [PATCH 2/4] Add liboffloadmic

2014-12-12 Thread Thomas Schwinge 
Hi!

On Tue, 21 Oct 2014 21:20:34 +0400, Ilya Verbin  wrote:
> This patch contains liboffloadmic library.

> liboffloadmic/
>   Initial commit.  Imported from upstream:
>   https://www.openmprtl.org/sites/default/files/liboffload_oss.tgz
>   * Makefile.am: New file.
>   * Makefile.in: New file, generated by automake.
>   * aclocal.m4: New file, generated by aclocal.
>   * configure: New file, generated by autoconf.
>   * configure.ac: New file.

contrib/gcc_update:files_and_dependencies needs to be updated for those
build machinery files as well as those added in liboffloadmic/plugin/
later on.


Grüße,
 Thomas


signature.asc
Description: PGP signature

Re: [PATCH 2/4] Add liboffloadmic

2014-11-12 Thread Ilya Verbin 
On 06 Nov 19:21, Jakub Jelinek wrote:
> I'm still seeing various unhandled malloc failures, e.g.:
> ...
> would crash if malloc returns NULL.  Similarly for realloc:

Fixed in the branch:
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=534b201fadf0af05ca509dc3f8e13ef26ee719a2

> Also, I see you heavily use malloc.h, not sure how portable that is,
> certainly e.g. gcc checks through configure for its presence.  I suppose
> stdlib.h being more portable.  Or, if you want liboffloadmic to be supported
> only on i?86-*-linux* / x86_64-*-linux* rather than all i?86/x86_64 targets,
> maybe you should say so in configure.tgt.  If you leave it on for all
> i?86/x86_64 targets, be prepared to handle issues on Darwin, mingw/cygwin,
> BSDs etc.

So far we have tested only i686/x86_64 linux, but probably we will support other
systems in the future.
I restricted $target to x86_64-*-linux* and i?86-*-linux* in configure.tgt.

On 12 Nov 11:40, Jakub Jelinek wrote:
> So, the whole series is ok to check in now.  Thanks.

Great, thank you!
We're going to check in all patches today after additional regtesting.

  -- Ilya

Re: [PATCH 2/4] Add liboffloadmic

2014-11-12 Thread Jakub Jelinek 
On Thu, Nov 06, 2014 at 07:21:31PM +0100, Jakub Jelinek wrote:
> On Wed, Oct 22, 2014 at 11:21:28PM +0400, Ilya Verbin wrote:
> > > Also, do we really want the messy DOS/Windows '\r' in the messages on
> > > Unix-ish targets?  Shouldn't that be dependent on what target is the 
> > > library
> > > configured for?
> > 
> > Fixed.
> 
> ...
> 
> I'm still seeing various unhandled malloc failures, e.g.:

Note, I'm ok with liboffloadmic being checked in right now as is and
these things fixed in it soon (within say 14 days from now).

So, the whole series is ok to check in now.  Thanks.

Jakub

Re: [PATCH 2/4] Add liboffloadmic

2014-11-06 Thread Jakub Jelinek 
On Wed, Oct 22, 2014 at 11:21:28PM +0400, Ilya Verbin wrote:
> > Also, do we really want the messy DOS/Windows '\r' in the messages on
> > Unix-ish targets?  Shouldn't that be dependent on what target is the library
> > configured for?
> 
> Fixed.

...

I'm still seeing various unhandled malloc failures, e.g.:

cean_util.cpp:res =(CeanReadRanges *)malloc(sizeof(CeanReadRanges) +
cean_util.cpp-  (ap->rank - rank) * 
sizeof(CeanReadDim));
cean_util.cpp-res->current_number = 0;
dv_util.cpp:res = (CeanReadRanges *)malloc(
dv_util.cpp-sizeof(CeanReadRanges) + (rank - i) * 
sizeof(CeanReadDim));
dv_util.cpp-res -> last_noncont_ind = rank - i - 1;
offload_env.cpp:env_var_def = (char*)malloc(sz);
offload_env.cpp-memcpy(env_var_def, env_var_name, sz);
offload_env.cpp-env_var_def[sz] = 0;
offload_env.cpp-int new_env_size = new_env.size();
offload_env.cpp:rez = (char**) malloc((new_env_size + 1) * sizeof(char*));
offload_env.cpp-std::copy(new_env.begin(), new_env.end(), rez);
offload_env.cpp-rez[new_env_size] = 0;
offload_host.cpp:char * ptr = (char*)malloc(size);
offload_host.cpp-COIRESULT res;
offload_host.cpp-
offload_host.cpp-memset(ptr, 0, size);
offload_host.cpp:m_vars = (VarDesc*) malloc(m_vars_total * 
sizeof(VarDesc));
offload_host.cpp-memcpy(m_vars, vars, m_vars_total * sizeof(VarDesc));
offload_host.cpp:m_func_desc = (FunctionDescriptor*) 
malloc(m_func_desc_size +
offload_host.cpp-   
misc_data_size);
offload_host.cpp-m_func_desc->console_enabled = console_enabled;
offload_host.cpp:res = (arr_desc *)malloc(sizeof(arr_desc));
offload_host.cpp-res->base = reinterpret_cast(ptr_val);

would crash if malloc returns NULL.  Similarly for realloc:

offload_host.cpp:m_vars   = (VarDesc*)realloc(m_vars, m_vars_total * 
sizeof(VarDesc));
offload_host.cpp-m_vars_extra =
offload_host.cpp:(VarExtra*)realloc(m_vars_extra, m_vars_total * 
sizeof(VarExtra));
...
offload_host.cpp-ext_elements.val = m_vars[i].count;

Also, I see you heavily use malloc.h, not sure how portable that is,
certainly e.g. gcc checks through configure for its presence.  I suppose
stdlib.h being more portable.  Or, if you want liboffloadmic to be supported
only on i?86-*-linux* / x86_64-*-linux* rather than all i?86/x86_64 targets,
maybe you should say so in configure.tgt.  If you leave it on for all
i?86/x86_64 targets, be prepared to handle issues on Darwin, mingw/cygwin,
BSDs etc.

Jakub

Re: [PATCH 2/4] Add liboffloadmic

2014-10-29 Thread Ilya Verbin 
On 22 Oct 23:21, Ilya Verbin wrote:
> On 22 Oct 10:54, Jakub Jelinek wrote:
> > On Tue, Oct 21, 2014 at 09:20:34PM +0400, Ilya Verbin wrote:
> > > This patch contains liboffloadmic library.
> > > 
> > > It is used by ICC for offloading.  The sources are imported from upstream
> > > ( https://www.openmprtl.org/sites/default/files/liboffload_oss.tgz )
> > > Configure and makefiles are new.
> > > 
> > > Also liboffloadmic/runtime/emulator directory is new.  This emulator 
> > > consists
> > > of 4 shared libraries which replace COI and MYO libraries from MPSS stack.
> > 
> > For the real offloading rather than emulation, are there any further
> > libraries needed, or are the COI and MYO bits included in the source
> > everything that is needed?  Does one need some kernel module, will it be
> > open source, are there any plans to push it to Linus' tree?
> > I know the HW isn't shipping yet, but it would be nice to get those
> > questions answered now.
> 
> In case of the real offloading, user is supposed to specify the path to real 
> COI
> and MYO libraries from MPSS:
> https://software.intel.com/en-us/articles/intel-manycore-platform-software-stack-mpss
> Their sources can be found in 'Downloads' section, in 'mpss-src-3.4.tar'.
> 
> COI in MYO depend only on the SCIF library, which is also open source.
> And SCIF requires some kernel module(s).  I'll ask the authors of MPSS, 
> whether
> all these modules are open source, and about their plans of pushing them to
> Linus' tree.

Yes, the kernel module required by SCIF is also open source.
(the sources are in mpss-modules-3.4.1.tar.bz2 inside mpss-src-3.4.1.tar)
And there are plans of pushing it to Linus' tree.

  -- Ilya

Re: [PATCH 2/4] Add liboffloadmic

2014-10-22 Thread Ilya Verbin 
On 22 Oct 10:54, Jakub Jelinek wrote:
> On Tue, Oct 21, 2014 at 09:20:34PM +0400, Ilya Verbin wrote:
> > This patch contains liboffloadmic library.
> > 
> > It is used by ICC for offloading.  The sources are imported from upstream
> > ( https://www.openmprtl.org/sites/default/files/liboffload_oss.tgz )
> > Configure and makefiles are new.
> > 
> > Also liboffloadmic/runtime/emulator directory is new.  This emulator 
> > consists
> > of 4 shared libraries which replace COI and MYO libraries from MPSS stack.
> 
> For the real offloading rather than emulation, are there any further
> libraries needed, or are the COI and MYO bits included in the source
> everything that is needed?  Does one need some kernel module, will it be
> open source, are there any plans to push it to Linus' tree?
> I know the HW isn't shipping yet, but it would be nice to get those
> questions answered now.

In case of the real offloading, user is supposed to specify the path to real COI
and MYO libraries from MPSS:
https://software.intel.com/en-us/articles/intel-manycore-platform-software-stack-mpss
Their sources can be found in 'Downloads' section, in 'mpss-src-3.4.tar'.

COI in MYO depend only on the SCIF library, which is also open source.
And SCIF requires some kernel module(s).  I'll ask the authors of MPSS, whether
all these modules are open source, and about their plans of pushing them to
Linus' tree.

> In git, I see that liboffloadmic/Makefile.am has -rwxrwxr-x permissions,
> please avoid that, only use executable permissions for shell scripts (so,
> configure is fine and desirable).

Fixed, I will update the patch in kyukhin/gomp4-offload git branch tomorrow, to
avoid resending the huge archive.

> From quick look at git, write_message has been supposedly fixed to use
> va_list, but I see it is wrong:
> 
> void write_message(FILE * file, int msgCode, va_list args_p) {
> va_list args;
> char buf[1024];
> 
> va_copy(args, args_p);
> buf[0] = '\r';
> vsnprintf(buf + 1, sizeof(buf), MESSAGE_TABLE_NAME[ msgCode ], args);
> strcat(buf, "\n");
> va_end(args);
> fputs(buf, file);
> fflush(file);
> }
> 
> As vsnprintf's first argument is buf + 1, it might happily overflow the
> buffer, and if not that, then the strcat could too.
> So, I'd expect sizeof(buf) - 2 be needed there instead of sizeof(buf).

Fixed.

> Also, do we really want the messy DOS/Windows '\r' in the messages on
> Unix-ish targets?  Shouldn't that be dependent on what target is the library
> configured for?

Fixed.

Thanks,
  -- Ilya

Re: [PATCH 2/4] Add liboffloadmic

2014-10-22 Thread Joseph S. Myers 
On Wed, 22 Oct 2014, Jakub Jelinek wrote:

> Also, do we really want the messy DOS/Windows '\r' in the messages on
> Unix-ish targets?  Shouldn't that be dependent on what target is the library
> configured for?

On platforms where it matters, I think it's still right to use \n only - 
if in the end something is output on a text stream, it's stdio's job to 
convert \n to \r\n as needed.

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [PATCH 2/4] Add liboffloadmic

2014-10-22 Thread Jakub Jelinek 
On Tue, Oct 21, 2014 at 09:20:34PM +0400, Ilya Verbin wrote:
> This patch contains liboffloadmic library.
> 
> It is used by ICC for offloading.  The sources are imported from upstream
> ( https://www.openmprtl.org/sites/default/files/liboffload_oss.tgz )
> Configure and makefiles are new.
> 
> Also liboffloadmic/runtime/emulator directory is new.  This emulator consists
> of 4 shared libraries which replace COI and MYO libraries from MPSS stack.

For the real offloading rather than emulation, are there any further
libraries needed, or are the COI and MYO bits included in the source
everything that is needed?  Does one need some kernel module, will it be
open source, are there any plans to push it to Linus' tree?
I know the HW isn't shipping yet, but it would be nice to get those
questions answered now.

In git, I see that liboffloadmic/Makefile.am has -rwxrwxr-x permissions,
please avoid that, only use executable permissions for shell scripts (so,
configure is fine and desirable).

>From quick look at git, write_message has been supposedly fixed to use
va_list, but I see it is wrong:

void write_message(FILE * file, int msgCode, va_list args_p) {
va_list args;
char buf[1024];

va_copy(args, args_p);
buf[0] = '\r';
vsnprintf(buf + 1, sizeof(buf), MESSAGE_TABLE_NAME[ msgCode ], args);
strcat(buf, "\n");
va_end(args);
fputs(buf, file);
fflush(file);
}

As vsnprintf's first argument is buf + 1, it might happily overflow the
buffer, and if not that, then the strcat could too.
So, I'd expect sizeof(buf) - 2 be needed there instead of sizeof(buf).
Also, do we really want the messy DOS/Windows '\r' in the messages on
Unix-ish targets?  Shouldn't that be dependent on what target is the library
configured for?

Jakub