Hello Jeff,
Please see my comments below:
Thanks,
Balaji V. Iyer.
> -----Original Message-----
> From: Jeff Law [mailto:l...@redhat.com]
> Sent: Tuesday, October 15, 2013 4:14 PM
> To: Iyer, Balaji V; gcc@gcc.gnu.org
> Cc: Aldy Hernandez (al...@redhat.com); r...@redhat.com; Jason Merrill
> (ja...@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.i...@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.i...@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
