c: Optimize TARGET_EXPRs for _Atomic loads [PR123475]

Message ID aWDe8FAx72rOBxoe@tucnak
State New
Headers
Series c: Optimize TARGET_EXPRs for _Atomic loads [PR123475] |

Commit Message

Jakub Jelinek Jan. 9, 2026, 10:56 a.m. UTC
  Hi!

On the following testcase we emit a false positive warning that
a temporary (TARGET_EXPR slot) is used uninitialized, from the early_uninit
pass.

This regressed with my change to instrument for
-ftrivial-auto-var-init={zero,pattern} not just DECL_EXPRs, but also
TARGET_EXPR initializations if the TARGET_EXPR_INITIALIZER has void type.
Those cases are where the initializer doesn't necessarily have to initialize
the whole TARGET_EXPR slot, or might use parts or the whole slot before
those are initialized; this is how e.g. various C++ temporary objects are
constructed.

The problem is in pass interaction.  The FE creates a TARGET_EXPR with
void type initializer because the initializer is originally
__atomic_load (&expr, &tmp, SEQ_CST); but it is folded instantly into
(void) (tmp = (type) __atomic_load_N (&expr, SEQ_CST)).  The FE also
marks the TARGET_EXPR slot as TREE_ADDRESSABLE, because it would be
if it will use libatomic, but nothing in the IL then takes its address.
Now, since my r16-4212 change which was for mainly C++26 compliance
we see the TARGET_EXPR and because it has void type TARGET_EXPR_INITIALIZER,
we start with tmp = .DEFERRED_INIT (...); just in case the initialization
would attempt to use the slot before initialization or not initialize fully.
Because tmp is TREE_ADDRESSABLE and has gimple reg type, it is actually not
gimplified as tmp = .DEFERRED_INIT (...); but as _1 = .DEFERRED_INIT (...);
tmp = _1; but because it is not actually address taken in the IL, already
the ssa pass turns it into SSA_NAME (dead one), so we have
_1 = .DEFERRED_INIT (...); _2 = _1; and _2 is unused.  Next comes
early_uninit and warns on the dead SSA_NAME copy that it uses uninitialized
var.

The following patch attempts to fix that by checking if
c_build_function_call_vec has optimized the call right away into pure
assignment to the TARGET_EXPR slot without the slot being used anywhere
else in the expression and 1) clearing again TREE_ADDRESSABLE on the slot,
because it isn't really addressable 2) optimizing the TARGET_EXPR, so that
it doesn't have void type TARGET_EXPR_INITIALIZER by changing it to the rhs
of the MODIFY_EXPR.  That way gimplifier doesn't bother creating
.DEFERRED_INIT for it at all.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Or should something like this be done instead in the TARGET_EXPR
gimplification?  I mean not the TREE_ADDRESSABLE clearing, that can't be
done without knowing what we know in the FE, but the rest, generally
TARGET_EXPR with initializer (void) (TARGET_EXPR_SLOT = something)
where something doesn't refer to TARGET_EXPR_SLOT can be optimized into
just something TARGET_EXPR_INITIALIZER.

2026-01-09  Jakub Jelinek  <jakub@redhat.com>

	PR c/123475
	* c-typeck.cc (c_find_var_r): New function.
	(convert_lvalue_to_rvalue): If c_build_function_call_vec
	folded __atomic_load (&expr, &tmp, SEQ_CST); into
	(void) (tmp = __atomic_load_<N> (&expr, SEQ_CST)), drop
	TREE_ADDRESSABLE flag from tmp and set TARGET_EXPR
	initializer just to the rhs of the MODIFY_EXPR.

	* gcc.dg/pr123475.c: New test.


	Jakub
  

Comments

Richard Biener Jan. 9, 2026, 11:08 a.m. UTC | #1
On Fri, 9 Jan 2026, Jakub Jelinek wrote:

> Hi!
> 
> On the following testcase we emit a false positive warning that
> a temporary (TARGET_EXPR slot) is used uninitialized, from the early_uninit
> pass.
> 
> This regressed with my change to instrument for
> -ftrivial-auto-var-init={zero,pattern} not just DECL_EXPRs, but also
> TARGET_EXPR initializations if the TARGET_EXPR_INITIALIZER has void type.
> Those cases are where the initializer doesn't necessarily have to initialize
> the whole TARGET_EXPR slot, or might use parts or the whole slot before
> those are initialized; this is how e.g. various C++ temporary objects are
> constructed.
> 
> The problem is in pass interaction.  The FE creates a TARGET_EXPR with
> void type initializer because the initializer is originally
> __atomic_load (&expr, &tmp, SEQ_CST); but it is folded instantly into
> (void) (tmp = (type) __atomic_load_N (&expr, SEQ_CST)).  The FE also
> marks the TARGET_EXPR slot as TREE_ADDRESSABLE, because it would be
> if it will use libatomic, but nothing in the IL then takes its address.
> Now, since my r16-4212 change which was for mainly C++26 compliance
> we see the TARGET_EXPR and because it has void type TARGET_EXPR_INITIALIZER,
> we start with tmp = .DEFERRED_INIT (...); just in case the initialization
> would attempt to use the slot before initialization or not initialize fully.
> Because tmp is TREE_ADDRESSABLE and has gimple reg type, it is actually not
> gimplified as tmp = .DEFERRED_INIT (...); but as _1 = .DEFERRED_INIT (...);
> tmp = _1; but because it is not actually address taken in the IL, already
> the ssa pass turns it into SSA_NAME (dead one), so we have
> _1 = .DEFERRED_INIT (...); _2 = _1; and _2 is unused.  Next comes
> early_uninit and warns on the dead SSA_NAME copy that it uses uninitialized
> var.
> 
> The following patch attempts to fix that by checking if
> c_build_function_call_vec has optimized the call right away into pure
> assignment to the TARGET_EXPR slot without the slot being used anywhere
> else in the expression and 1) clearing again TREE_ADDRESSABLE on the slot,
> because it isn't really addressable 2) optimizing the TARGET_EXPR, so that
> it doesn't have void type TARGET_EXPR_INITIALIZER by changing it to the rhs
> of the MODIFY_EXPR.  That way gimplifier doesn't bother creating
> .DEFERRED_INIT for it at all.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> Or should something like this be done instead in the TARGET_EXPR
> gimplification?  I mean not the TREE_ADDRESSABLE clearing, that can't be
> done without knowing what we know in the FE, but the rest, generally
> TARGET_EXPR with initializer (void) (TARGET_EXPR_SLOT = something)
> where something doesn't refer to TARGET_EXPR_SLOT can be optimized into
> just something TARGET_EXPR_INITIALIZER.

It feels like this simple situation is easy to avoid where created
as your patch does.  gimplification should in general not bother
about optimizing things that have a valid optimized representation
in GENERIC.  Possibly a build_target_expr helper could though?

Richard.

> 2026-01-09  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c/123475
> 	* c-typeck.cc (c_find_var_r): New function.
> 	(convert_lvalue_to_rvalue): If c_build_function_call_vec
> 	folded __atomic_load (&expr, &tmp, SEQ_CST); into
> 	(void) (tmp = __atomic_load_<N> (&expr, SEQ_CST)), drop
> 	TREE_ADDRESSABLE flag from tmp and set TARGET_EXPR
> 	initializer just to the rhs of the MODIFY_EXPR.
> 
> 	* gcc.dg/pr123475.c: New test.
> 
> --- gcc/c/c-typeck.cc.jj	2026-01-03 12:11:50.511872222 +0100
> +++ gcc/c/c-typeck.cc	2026-01-08 19:15:35.748238143 +0100
> @@ -2590,6 +2590,19 @@ maybe_get_constexpr_init (tree expr)
>    return build_zero_cst (TREE_TYPE (expr));
>  }
>  
> +/* Helper function for convert_lvalue_to_rvalue called via
> +   walk_tree_without_duplicates.  Find DATA inside of the expression.  */
> +
> +static tree
> +c_find_var_r (tree *tp, int *walk_subtrees, void *data)
> +{
> +  if (TYPE_P (*tp))
> +    *walk_subtrees = 0;
> +  else if (*tp == (tree) data)
> +    return *tp;
> +  return NULL_TREE;
> +}
> +
>  /* Convert expression EXP (location LOC) from lvalue to rvalue,
>     including converting functions and arrays to pointers if CONVERT_P.
>     If READ_P, also mark the expression as having been read.  If
> @@ -2664,6 +2677,25 @@ convert_lvalue_to_rvalue (location_t loc
>        /* EXPR is always read.  */
>        mark_exp_read (exp.value);
>  
> +      /* Optimize the common case where c_build_function_call_vec
> +	 immediately folds __atomic_load (&expr, &tmp, SEQ_CST); into
> +	 tmp = __atomic_load_<N> (&expr, SEQ_CST);
> +	 In that case tmp is not addressable and can be initialized
> +	 fully by the rhs of the MODIFY_EXPR.  */
> +      tree tem = func_call;
> +      if (CONVERT_EXPR_P (tem) && VOID_TYPE_P (TREE_TYPE (tem)))
> +	{
> +	  tem = TREE_OPERAND (tem, 0);
> +	  if (TREE_CODE (tem) == MODIFY_EXPR
> +	      && TREE_OPERAND (tem, 0) == tmp
> +	      && !walk_tree_without_duplicates (&TREE_OPERAND (tem, 1),
> +						c_find_var_r, tmp))
> +	    {
> +	      TREE_ADDRESSABLE (tmp) = 0;
> +	      func_call = TREE_OPERAND (tem, 1);
> +	    }
> +	}
> +
>        /* Return tmp which contains the value loaded.  */
>        exp.value = build4 (TARGET_EXPR, nonatomic_type, tmp, func_call,
>  			  NULL_TREE, NULL_TREE);
> --- gcc/testsuite/gcc.dg/pr123475.c.jj	2026-01-08 19:28:35.348556652 +0100
> +++ gcc/testsuite/gcc.dg/pr123475.c	2026-01-08 19:28:09.213005228 +0100
> @@ -0,0 +1,12 @@
> +/* PR c/123475 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -Wuninitialized -ftrivial-auto-var-init=zero" } */
> +
> +int
> +foo (void)
> +{
> +  _Atomic int a = 1000;
> +  int b;
> +  b = a;	/* { dg-bogus "is used uninitialized" } */
> +  return b;
> +}
> 
> 	Jakub
> 
>
  
Joseph Myers Jan. 9, 2026, 7:10 p.m. UTC | #2
On Fri, 9 Jan 2026, Jakub Jelinek wrote:

> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.
  

Patch

--- gcc/c/c-typeck.cc.jj	2026-01-03 12:11:50.511872222 +0100
+++ gcc/c/c-typeck.cc	2026-01-08 19:15:35.748238143 +0100
@@ -2590,6 +2590,19 @@  maybe_get_constexpr_init (tree expr)
   return build_zero_cst (TREE_TYPE (expr));
 }
 
+/* Helper function for convert_lvalue_to_rvalue called via
+   walk_tree_without_duplicates.  Find DATA inside of the expression.  */
+
+static tree
+c_find_var_r (tree *tp, int *walk_subtrees, void *data)
+{
+  if (TYPE_P (*tp))
+    *walk_subtrees = 0;
+  else if (*tp == (tree) data)
+    return *tp;
+  return NULL_TREE;
+}
+
 /* Convert expression EXP (location LOC) from lvalue to rvalue,
    including converting functions and arrays to pointers if CONVERT_P.
    If READ_P, also mark the expression as having been read.  If
@@ -2664,6 +2677,25 @@  convert_lvalue_to_rvalue (location_t loc
       /* EXPR is always read.  */
       mark_exp_read (exp.value);
 
+      /* Optimize the common case where c_build_function_call_vec
+	 immediately folds __atomic_load (&expr, &tmp, SEQ_CST); into
+	 tmp = __atomic_load_<N> (&expr, SEQ_CST);
+	 In that case tmp is not addressable and can be initialized
+	 fully by the rhs of the MODIFY_EXPR.  */
+      tree tem = func_call;
+      if (CONVERT_EXPR_P (tem) && VOID_TYPE_P (TREE_TYPE (tem)))
+	{
+	  tem = TREE_OPERAND (tem, 0);
+	  if (TREE_CODE (tem) == MODIFY_EXPR
+	      && TREE_OPERAND (tem, 0) == tmp
+	      && !walk_tree_without_duplicates (&TREE_OPERAND (tem, 1),
+						c_find_var_r, tmp))
+	    {
+	      TREE_ADDRESSABLE (tmp) = 0;
+	      func_call = TREE_OPERAND (tem, 1);
+	    }
+	}
+
       /* Return tmp which contains the value loaded.  */
       exp.value = build4 (TARGET_EXPR, nonatomic_type, tmp, func_call,
 			  NULL_TREE, NULL_TREE);
--- gcc/testsuite/gcc.dg/pr123475.c.jj	2026-01-08 19:28:35.348556652 +0100
+++ gcc/testsuite/gcc.dg/pr123475.c	2026-01-08 19:28:09.213005228 +0100
@@ -0,0 +1,12 @@ 
+/* PR c/123475 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wuninitialized -ftrivial-auto-var-init=zero" } */
+
+int
+foo (void)
+{
+  _Atomic int a = 1000;
+  int b;
+  b = a;	/* { dg-bogus "is used uninitialized" } */
+  return b;
+}