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

Reply via email to