Re: Cilk Library

[Richard Biener <richard dot guenther at gmail dot com>]

On Tue, Oct 15, 2013 at 10:14 PM, Jeff Law <law@redhat.com> wrote:
> 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=sharing).
>>
>>       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.
>
> 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.
>
> 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.

As RM I strongly object to adding a runtime library that does not apply
basic symbol export control and versioning.  If you are restricting yourselves
to x86-linux for now anyway it should be pretty easy to setup a linker
version script for this.

Richard.

> 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?
>
> Does this runtime include the complete cilkscreen race detector?  Was that
> intentional?
>
> 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
>
> Is this make -j clean to the best of your knowledge?
>
>
> Jeff