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

[Ramana Radhakrishnan <ramana dot radhakrishnan at foss dot arm dot com>]

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.


regards
Ramana

Attachment: fenv-pr65354.txt
Description: Text document