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

[David Edelsohn <dje.gcc@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2401 bytes --]

On Fri, Oct 2, 2015 at 1:08 PM, Ramana Radhakrishnan
<ramana.radhakrishnan@foss.arm.com> wrote:

>> 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 ....

The bug was not x86-specific.  The fix happens to be in
target-specific code, but that's the luck of the draw.  Numerous other
GCC developers have fixed bugs or added features that required tweaks
to all ports.  Not all targets are easily accessible and you certainly
can ask port maintainers to test a patch.  But writing that you don't
have the cycles to fix all of the targets is not a collegial answer.
Why do you believe that target maintainers have more cycles than you?
I didn't see you tell Uros to fix the bug on x86.  The approach may
work for your one specific bug, but it does not scale if every GCC
developer pursues the same process.

It's poor form to fix a bug only on x86 that is common to all targets
and leave the problem "as an exercise for the reader" for all other
targets -- essentially banishing the other targets to second-class
status with respect to conformance -- especially when the change is
mostly mechanical.  I don't expect developers to specifically enable
and exploit every new feature on every architecture, but had expected
bug fixes to be distributed to all targets.  "It's just not cricket."

GCC has thrived for over 25 years -- supporting a huge number of
targets and languages -- through a general sense of cooperation and
collaboration to ensure the success of the entire project.  If this is
going to degrade into a more parochial attitude, then maybe GCC will
need an explicit policy to counteract that mindset.

I am testing the attached patch for PPC.

- David

[-- Attachment #2: ZZ --]
[-- Type: application/octet-stream, Size: 1682 bytes --]

Index: rs6000.c
===================================================================
--- rs6000.c	(revision 228451)
+++ rs6000.c	(working copy)
@@ -36489,7 +36489,7 @@ rs6000_atomic_assign_expand_fenv (tree *hold, tree
 	  DECL_EXTERNAL (atomic_update_decl) = 1;
 	}
 
-      tree fenv_var = create_tmp_var (double_type_node);
+      tree fenv_var = create_tmp_var_raw (double_type_node);
       mark_addressable (fenv_var);
       tree fenv_addr = build1 (ADDR_EXPR, double_ptr_type_node, fenv_var);
 
@@ -36517,7 +36517,7 @@ rs6000_atomic_assign_expand_fenv (tree *hold, tree
   const unsigned HOST_WIDE_INT hold_exception_mask =
     HOST_WIDE_INT_C (0xffffffff00000007);
 
-  tree fenv_var = create_tmp_var (double_type_node);
+  tree fenv_var = create_tmp_var_raw (double_type_node);
 
   tree hold_mffs = build2 (MODIFY_EXPR, void_type_node, fenv_var, call_mffs);
 
@@ -36546,7 +36546,7 @@ rs6000_atomic_assign_expand_fenv (tree *hold, tree
   const unsigned HOST_WIDE_INT clear_exception_mask =
     HOST_WIDE_INT_C (0xffffffff00000000);
 
-  tree fenv_clear = create_tmp_var (double_type_node);
+  tree fenv_clear = create_tmp_var_raw (double_type_node);
 
   tree clear_mffs = build2 (MODIFY_EXPR, void_type_node, fenv_clear, call_mffs);
 
@@ -36578,7 +36578,7 @@ rs6000_atomic_assign_expand_fenv (tree *hold, tree
   const unsigned HOST_WIDE_INT new_exception_mask =
     HOST_WIDE_INT_C (0x1ff80fff);
 
-  tree old_fenv = create_tmp_var (double_type_node);
+  tree old_fenv = create_tmp_var_raw (double_type_node);
   tree update_mffs = build2 (MODIFY_EXPR, void_type_node, old_fenv, call_mffs);
 
   tree old_llu = build1 (VIEW_CONVERT_EXPR, uint64_type_node, old_fenv);