This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: C PATCH for c/65345 (file-scope _Atomic expansion with floats)


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
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]