This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][CilkPlus] Merge libcilkrts from upstream
- From: Christophe Lyon <christophe dot lyon at linaro dot org>
- To: Ilya Verbin <iverbin at gmail dot com>
- Cc: Ramana Radhakrishnan <ramana dot gcc at googlemail dot com>, Jeff Law <law at redhat dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, Kirill Yukhin <kirill dot yukhin at gmail dot com>
- Date: Tue, 10 May 2016 21:24:07 +0200
- Subject: Re: [PATCH][CilkPlus] Merge libcilkrts from upstream
- Authentication-results: sourceware.org; auth=none
- References: <20160429113631 dot GA27336 at msticlxl57 dot ims dot intel dot com> <6a243979-c9c9-ba5d-d41b-e0c57917e82d at redhat dot com> <CAKdteOZNVbWxkw=VcBbgDPznP06saHFth_mNpSWhMKQ6oLv1cQ at mail dot gmail dot com> <d67a7f33-9eff-65e7-9404-922f9f46cedd at redhat dot com> <CAKdteOankELyBJkxNOWSxG3NLFyRwHgreb6ba9As4o0qz3fZjg at mail dot gmail dot com> <CAKdteOaeWFKw+XfyCshS+9iMm1dQMVmfKfRU66wgN+KwX6JUAA at mail dot gmail dot com> <CAJA7tRaxcWKjn=9yFfrxPSY4VDFjuQtG=U7vO-nc6Ow+9tpTaQ at mail dot gmail dot com> <20160510171848 dot GC47195 at msticlxl57 dot ims dot intel dot com>
On 10 May 2016 at 19:18, Ilya Verbin <iverbin@gmail.com> wrote:
> On Tue, May 10, 2016 at 14:36:36 +0100, Ramana Radhakrishnan wrote:
>> On Tue, May 10, 2016 at 2:02 PM, Christophe Lyon
>> <christophe.lyon@linaro.org> wrote:
>> > On 9 May 2016 at 15:34, Christophe Lyon <christophe.lyon@linaro.org> wrote:
>> >> On 9 May 2016 at 15:29, Jeff Law <law@redhat.com> wrote:
>> >>> On 05/09/2016 01:37 AM, Christophe Lyon wrote:
>> >>>> After this merge, I'm seeing lots of timeouts on arm (using QEMU).
>> >>>> Is this "expected"? (as in: should I increase my timeout value)
>> >>>
>> >>> I wouldn't say it's expected; this is the first time Cilk+ has been
>> >>> supported on ARM. It could be a bug in the ARM support in the runtime, an
>> >>> ARM compiler bug or even a bug in the ARM QEMU support.
>> >>>
>> >>> Probably the first step is to see if it's working properly on real hardware.
>> >>> That would at least allow us to eliminate QEMU from the equation if it's
>> >>> failing in the same manner on a real machine.
>> >>>
>> >> OK, I'll check that.
>> >> I wanted to know if I was missing something obvious.
>> >
>> > I've tested in an armhf chroot on an armv8 machine, and I saw SIGILL errors
>> > on:
>> > mcr 15, 0, r3, cr7, cr10, {4}
>> > which is how __cilkrts_fence is implemented in
>> > ../libcilkrts/runtime/config/arm/os-fence.h
>>
>> At first glance I'd ask why this shouldn't be __atomic_thread_fence or
>> __atomic_signal_fence ( SEQ_CST) if that's what they want here and
>> then it will work (TM) regardless of architecture levels.
>>
>> > This instruction is not supported anymore on armv8. Recent arm64 kernels
>> > have handlers for it.
>> >
>> > So we may want the implementation to be conditional, or prefer to rely on
>> > kernel support.
>
> ARM enabling code was taken from community contribution, we haven't tested it.
> If someone wants to fix this, it would be appreciated.
>
Following Ramana's suggestion, I tried:
# define __cilkrts_fence() __atomic_thread_fence(__ATOMIC_SEQ_CST);
and the tests now pass.
Christophe
> Thanks,
> -- Ilya