On 10 May 2016 at 21:24, Christophe Lyon <christophe.l...@linaro.org> wrote:
> On 10 May 2016 at 19:18, Ilya Verbin <iver...@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.l...@linaro.org> wrote:
>>> > On 9 May 2016 at 15:34, Christophe Lyon <christophe.l...@linaro.org>
>>> > wrote:
>>> >> On 9 May 2016 at 15:29, Jeff Law <l...@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.
>
I've looked at the generated code in more details, and for armv6 this generates
mcr p15, 0, r0, c7, c10, 5
which is not what __cilkrts_fence uses currently (CP15DSB vs CP15DMB)
Looking at arm/sync.md it seems that there is no way to generate CP15DSB.
> Christophe
>
>> Thanks,
>> -- Ilya
