RE: Cilk Library

["Iyer, Balaji V" <balaji dot v dot iyer at intel dot com>]

Hello Jeff,	
	Please see my comments below:

Thanks,

Balaji V. Iyer.


> -----Original Message-----
> From: Jeff Law [mailto:law@redhat.com]
> Sent: Tuesday, October 15, 2013 4:14 PM
> To: Iyer, Balaji V; gcc@gcc.gnu.org
> Cc: Aldy Hernandez (aldyh@redhat.com); rth@redhat.com; Jason Merrill
> (jason@redhat.com)
> Subject: Re: Cilk Library
> 
> On 10/09/13 12:32, Iyer, Balaji V wrote:
> > Dear Jeff and the rest of Steering committee members,
> >       Thank you very much for approving the license terms of the Cilk Library. I
> couldn't attach the zipped copy of the patch due to its size, so here is a link to
> the Cilk library patch that can be applied to the trunk:
> (https://docs.google.com/file/d/0BzEpbbnrYKsSWjBWSkNrVS1SaGs/edit?usp=sh
> aring).
> >
> >       Is it OK for trunk?
> >
> > Here are the ChangeLog entries:
> >
> > ChangeLog:
> > 2013-10-09  Balaji V. Iyer  <balaji.v.iyer@intel.com>
> >
> >          * Makefile.def: Add libcilkrts to target_modules.  Make libcilkrts
> >          depend on libstdc++ and libgcc.
> >          * configure.ac: Added libcilkrts to target binaries.
> >          * configure: Likewise.
> >          * Makefile.in: Added libcilkrts related fields to support building it.
> >
> > libcilkrts/ChangeLog:
> > 2013-10-09  Balaji V. Iyer  <balaji.v.iyer@intel.com>
> >
> > 	* libcilkrts/Makefile.am: New file.  Libcilkrts version 3613.
> > 	* libcilkrts/Makefile.in: Likewise.
> > 	* libcilkrts/README: Likewise.
> > 	* libcilkrts/aclocal.m4: Likewise.
> > 	* libcilkrts/configure: Likewise.
> > 	* libcilkrts/configure.ac: Likewise.
> > 	* libcilkrts/include/cilk/cilk.h: Likewise.
> > 	* libcilkrts/include/cilk/cilk_api.h: Likewise.
> > 	* libcilkrts/include/cilk/cilk_api_linux.h: Likewise.
> > 	* libcilkrts/include/cilk/cilk_stub.h: Likewise.
> > 	* libcilkrts/include/cilk/cilk_undocumented.h: Likewise.
> > 	* libcilkrts/include/cilk/common.h: Likewise.
> > 	* libcilkrts/include/cilk/holder.h: Likewise.
> > 	* libcilkrts/include/cilk/hyperobject_base.h: Likewise.
> > 	* libcilkrts/include/cilk/metaprogramming.h: Likewise.
> > 	* libcilkrts/include/cilk/reducer.h: Likewise.
> > 	* libcilkrts/include/cilk/reducer_file.h: Likewise.
> > 	* libcilkrts/include/cilk/reducer_list.h: Likewise.
> > 	* libcilkrts/include/cilk/reducer_max.h: Likewise.
> > 	* libcilkrts/include/cilk/reducer_min.h: Likewise.
> > 	* libcilkrts/include/cilk/reducer_min_max.h: Likewise.
> > 	* libcilkrts/include/cilk/reducer_opadd.h: Likewise.
> > 	* libcilkrts/include/cilk/reducer_opand.h: Likewise.
> > 	* libcilkrts/include/cilk/reducer_opmul.h: Likewise.
> > 	* libcilkrts/include/cilk/reducer_opor.h: Likewise.
> > 	* libcilkrts/include/cilk/reducer_opxor.h: Likewise.
> > 	* libcilkrts/include/cilk/reducer_ostream.h: Likewise.
> > 	* libcilkrts/include/cilk/reducer_string.h: Likewise.
> > 	* libcilkrts/include/cilktools/cilkscreen.h: Likewise.
> > 	* libcilkrts/include/cilktools/cilkview.h: Likewise.
> > 	* libcilkrts/include/cilktools/fake_mutex.h: Likewise.
> > 	* libcilkrts/include/cilktools/lock_guard.h: Likewise.
> > 	* libcilkrts/include/internal/abi.h: Likewise.
> > 	* libcilkrts/include/internal/cilk_fake.h: Likewise.
> > 	* libcilkrts/include/internal/cilk_version.h: Likewise.
> > 	* libcilkrts/include/internal/inspector-abi.h: Likewise.
> > 	* libcilkrts/include/internal/metacall.h: Likewise.
> > 	* libcilkrts/include/internal/rev.mk: Likewise.
> > 	* libcilkrts/mk/cilk-version.mk: Likewise.
> > 	* libcilkrts/mk/unix-common.mk: Likewise.
> > 	* libcilkrts/runtime/acknowledgements.dox: Likewise.
> > 	* libcilkrts/runtime/bug.cpp: Likewise.
> > 	* libcilkrts/runtime/bug.h: Likewise.
> > 	* libcilkrts/runtime/c_reducers.c: Likewise.
> > 	* libcilkrts/runtime/cilk-abi-cilk-for.cpp: Likewise.
> > 	* libcilkrts/runtime/cilk-abi-vla-internal.c: Likewise.
> > 	* libcilkrts/runtime/cilk-abi-vla-internal.h: Likewise.
> > 	* libcilkrts/runtime/cilk-abi-vla.c: Likewise.
> > 	* libcilkrts/runtime/cilk-abi.c: Likewise.
> > 	* libcilkrts/runtime/cilk-ittnotify.h: Likewise.
> > 	* libcilkrts/runtime/cilk-tbb-interop.h: Likewise.
> > 	* libcilkrts/runtime/cilk_api.c: Likewise.
> > 	* libcilkrts/runtime/cilk_fiber-unix.cpp: Likewise.
> > 	* libcilkrts/runtime/cilk_fiber-unix.h: Likewise.
> > 	* libcilkrts/runtime/cilk_fiber.cpp: Likewise.
> > 	* libcilkrts/runtime/cilk_fiber.h: Likewise.
> > 	* libcilkrts/runtime/cilk_malloc.c: Likewise.
> > 	* libcilkrts/runtime/cilk_malloc.h: Likewise.
> > 	* libcilkrts/runtime/component.h: Likewise.
> > 	* libcilkrts/runtime/doxygen-layout.xml: Likewise.
> > 	* libcilkrts/runtime/doxygen.cfg: Likewise.
> > 	* libcilkrts/runtime/except-gcc.cpp: Likewise.
> > 	* libcilkrts/runtime/except-gcc.h: Likewise.
> > 	* libcilkrts/runtime/except.h: Likewise.
> > 	* libcilkrts/runtime/frame_malloc.c: Likewise.
> > 	* libcilkrts/runtime/frame_malloc.h: Likewise.
> > 	* libcilkrts/runtime/full_frame.c: Likewise.
> > 	* libcilkrts/runtime/full_frame.h: Likewise.
> > 	* libcilkrts/runtime/global_state.cpp: Likewise.
> > 	* libcilkrts/runtime/global_state.h: Likewise.
> > 	* libcilkrts/runtime/jmpbuf.c: Likewise.
> > 	* libcilkrts/runtime/jmpbuf.h: Likewise.
> > 	* libcilkrts/runtime/local_state.c: Likewise.
> > 	* libcilkrts/runtime/local_state.h: Likewise.
> > 	* libcilkrts/runtime/metacall_impl.c: Likewise.
> > 	* libcilkrts/runtime/metacall_impl.h: Likewise.
> > 	* libcilkrts/runtime/os-unix.c: Likewise.
> > 	* libcilkrts/runtime/os.h: Likewise.
> > 	* libcilkrts/runtime/os_mutex-unix.c: Likewise.
> > 	* libcilkrts/runtime/os_mutex.h: Likewise.
> > 	* libcilkrts/runtime/pedigrees.c: Likewise.
> > 	* libcilkrts/runtime/pedigrees.h: Likewise.
> > 	* libcilkrts/runtime/record-replay.cpp: Likewise.
> > 	* libcilkrts/runtime/record-replay.h: Likewise.
> > 	* libcilkrts/runtime/reducer_impl.cpp: Likewise.
> > 	* libcilkrts/runtime/reducer_impl.h: Likewise.
> > 	* libcilkrts/runtime/rts-common.h: Likewise.
> > 	* libcilkrts/runtime/scheduler.c: Likewise.
> > 	* libcilkrts/runtime/scheduler.h: Likewise.
> > 	* libcilkrts/runtime/signal_node.c: Likewise.
> > 	* libcilkrts/runtime/signal_node.h: Likewise.
> > 	* libcilkrts/runtime/spin_mutex.c: Likewise.
> > 	* libcilkrts/runtime/spin_mutex.h: Likewise.
> > 	* libcilkrts/runtime/stacks.h: Likewise.
> > 	* libcilkrts/runtime/stats.c: Likewise.
> > 	* libcilkrts/runtime/stats.h: Likewise.
> > 	* libcilkrts/runtime/symbol_test.c: Likewise.
> > 	* libcilkrts/runtime/sysdep-unix.c: Likewise.
> > 	* libcilkrts/runtime/sysdep.h: Likewise.
> > 	* libcilkrts/runtime/unix_symbols.t: Likewise.
> > 	* libcilkrts/runtime/worker_mutex.c: Likewise.
> > 	* libcilkrts/runtime/worker_mutex.h: Likewise.
> >
> > Thanks,
> First, the all issues Joseph mentioned need to be addressed.  So first, you need
> to ensure it's only being built on x86/x86_64 given the asms and bring together
> some documentation as to what's needed to port the
> runtime system to other architectures.   Closely related, I think you
> initially need to ensure it only builds on x86-linux platforms -- unless you've
> already verified it works properly on one or more of the bsd platforms, solaris,
> windows, etc.
> 

We are in the process of addressing all these. I will send out an email with the fixed runtime as soon as I can.

> In the README, you'll want to update the reference to the cilkplus branch to
> something like GCC 4.9 or later since presumably the cilkplus branch will be
> retired once all the bits are in place.  It also mentions the GPL, so those bits are
> probably out of date given the plan to use the 3-clause BSD license, similarly for
> various other files.  A grep for GPL might be in order.
> 

Yup, this is caught and fixed in the Readme. The newer one that we will send shortly should reflect this.

> I didn't look at the make/configury closely.  It just makes my head hurt.  I'm
> going to assume nothing is terribly busted in there other than the need
> conditionalize properly per Joseph's comments, avoidance of DATE, TIME, and
> the like.
> 
> I would strongly echo Joseph's recommendation to ensure that only those
> symbols specifically intended to be part of the public interface are exported
> from the shared library. How stable has the exported API/ABI
> for Cilk+ been?    Related: how clean is the RTS from a compile-time
> namespace pollution standpoint.  For C++ is everything in a namespace, for C is
> everything prefixed appropriately?
> 

We are currently verifying those and making sure only the required symbols are exported.

> Does this runtime include the complete cilkscreen race detector?  Was that
> intentional?
> 

Cilkscreen binaries are available for free from our website (www.cilkplus.org). To use Cilkscreen, compiler must insert some information about location of Spawn, continuation, sync points, etc. into the executable. I am hoping to submit those changes as a separate patch after I submit all the Cilk Plus features.

> As I mentioned, we're not going to nitpick on the actual source of the runtime
> system because it's being maintained upstream by Intel.  So all the things which
> hurt my eyes when I scanned the library code, I'll just ignore.
> 
> Since this is being maintained upstream, a notice where to send any
> contributions would be wise.  Look at libsanitizer/README.gcc.
> 
> I'm assuming the multilib stuff works... :-0
> 

Yes, multilib works correctly. 


> Is this make -j clean to the best of your knowledge?
> 
> 

Yes, make -j8 seem to work well for me when I use it in the Cilkplus branch.

> Jeff