This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: C PATCH for c/65345 (file-scope _Atomic expansion with floats)
- From: Christophe Lyon <christophe dot lyon at linaro dot org>
- To: Ramana Radhakrishnan <ramana dot radhakrishnan at foss dot arm dot com>
- Cc: Marek Polacek <polacek at redhat dot com>, David Edelsohn <dje dot gcc at gmail dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>, Joseph Myers <joseph at codesourcery dot com>, Catherine Moore <clm at codesourcery dot com>, Matthew Fortune <matthew dot fortune at imgtec dot com>, Eric Botcazou <ebotcazou at adacore dot com>, Richard Henderson <rth at redhat dot com>, Uros Bizjak <ubizjak at gmail dot com>, David Miller <davem at redhat dot com>, Ulrich Weigand <uweigand at de dot ibm dot com>, Andreas Krebbel <Andreas dot Krebbel at de dot ibm dot com>, Richard Earnshaw <Richard dot Earnshaw at arm dot com>, "nickc at redhat dot com" <nickc at redhat dot com>, "olegendo at gcc dot gnu dot org" <olegendo at gcc dot gnu dot org>, "kkojima at gcc dot gnu dot org" <kkojima at gcc dot gnu dot org>, Marcus Shawcroft <Marcus dot Shawcroft at arm dot com>
- Date: Mon, 5 Oct 2015 12:00:20 +0200
- Subject: Re: C PATCH for c/65345 (file-scope _Atomic expansion with floats)
- Authentication-results: sourceware.org; auth=none
- References: <20151001144941 dot GT6184 at redhat dot com> <CAGWvnyksEqPaLLQm-ieuE0XF_F9nS5Q1nVMyvp7SMtp5kBA_aw at mail dot gmail dot com> <20151001161834 dot GU6184 at redhat dot com> <560EB9FB dot 1010003 at foss dot arm dot com>
On 2 October 2015 at 19:08, Ramana Radhakrishnan
<ramana.radhakrishnan@foss.arm.com> wrote:
>
>
> On 01/10/15 17:18, Marek Polacek wrote:
>> On Thu, Oct 01, 2015 at 11:02:09AM -0400, David Edelsohn wrote:
>>> On Thu, Oct 1, 2015 at 10:49 AM, Marek Polacek <polacek@redhat.com> wrote:
>>>> Joseph reminded me that I had forgotten about this patch. As mentioned
>>>> here <https://gcc.gnu.org/ml/gcc-patches/2015-04/msg01792.html>, I'm
>>>> removing the XFAILs in the tests so people are likely to see new FAILs.
>>>>
>>>> I think the following targets will need similar fix as the one below:
>>>> * MIPS
>>>> * rs6000
>>>> * alpha
>>>> * sparc
>>>> * s390
>>>> * arm
>>>> * sh
>>>> * aarch64
>>>>
>>>> I'm CCing the respective maintainers. You might want to XFAIL those tests.
>
> Thanks for the heads up. The use of the _Atomic feature
> is not really target specific but more a language standards issue across all the architectures
> that the GCC project supports, therefore XFAILing them is the wrong approach imho.
>
>
>>>
>>> Why aren't you testing the appropriate fix on all of the targets?
>>
>> It's very improbable that I could fix and properly test all of them;
>> I simply don't have the cycles and resources to fix e.g. sh/sparc/alpha/mips.
>
> I don't think anyone expects you to be testing the patch on every single port .....
>
> Even though these changes sit in the target hooks into various backends, you may be best
> placed to advise how target maintainers adjust their backends. If at that point this appears to be
> mechanical, it's been good practice in the community for folks to send patches
> that the maintainers can fully test even if the testing has been light for the
> proposed patch.
>
> However, I am not aware of a "policy" for these things other than that these
> sort of changes are selectively enforced in the community. Maybe we should think
> about it ....
>
>
>>
>> You want me to revert my fix, but I don't really see the point here; the
>> patch doesn't introduce any regressions, it's just that the new tests are
>> likely to FAIL. It sounds preferable to me to fix 2 targets than to leave
>> all of them broken (and I bet many maintainers were unaware of the issue).
>>
>
>
>> Would XFAILing the new tests work for you, if you don't want to see any
>> new FAILs?
>>
>> If you still insist on reverting the patch, ok, but I think this PR is
>> unlikely to be resolved any time soon then.
>>
>> Marek
>>
>
>
> I've had a quick look on aarch64 - changing the interface to use create_tmp_var_raw
> is rather mechanical. What I'm struggling with is figuring out whether
> the change for TARGET_EXPR is applicable in the arm / aarch64 backends.
>
> It took me a couple of minutes to trial the interface changes (attached) on aarch64
> as I had a cross-compiler build tree lying around and could see that the compiler
> did not ICE with the 2 testcases provided and pr65345-4.c appeared to pass on hardware.
>
>
I've just created https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67848
which covers aarch64 and arm-linux-gnueabihf targets.
> regards
> Ramana
>