diff mbox series

[C] fix atomic loads [PR 97981]

Message ID 1607197513.5452.4.camel@med.uni-goettingen.de
State New
Headers show
Series [C] fix atomic loads [PR 97981] | expand

Commit Message

Uecker, Martin Dec. 5, 2020, 7:45 p.m. UTC 
Hi Joseph,

the patch to drop qualifiers during lvalue conversion
was broken, because the code to emit atomic loads did
not trigger anymore.  I now added a test that scans for
"atomic_load". 

I should have taken the new warning for

_Atomic int y;
y; // warning statement with no effect 

as a tell-tale sign that something is wrong,
although I still think the warning would be
correct. Or has a atomic load some special
semantics which imply that it can't be
removed?

(regression tests still running)


--Martin




C: Fix atomic loads. [PR97981]

To handle atomic loads correctly, we need to move the code
top qualifiers in lvalue conversion after
the code that
handles atomics.

    
2020-12-05  Martin Uecker  <muecker@gwdg.de>
    
gcc/c/
     PR c/97981
     * c-typeck.c (convert_lvalue_to_rvalue): Move the code
     that drops qualifiers to the end of the function.      

gcc/testsuite/
     PR c/97981
     * gcc.dg/pr97981.c: New test.
     * gcc.dg/pr60195.c. Adapt test.

Comments

Joseph Myers Dec. 7, 2020, 10:36 p.m. UTC | #1 
On Sat, 5 Dec 2020, Uecker, Martin wrote:

> I should have taken the new warning for
> 
> _Atomic int y;
> y; // warning statement with no effect 
> 
> as a tell-tale sign that something is wrong,
> although I still think the warning would be
> correct. Or has a atomic load some special
> semantics which imply that it can't be
> removed?

The warning is logically correct for an atomic load (even if in some cases 
it's built on primitives that require write access and don't work for a 
load from read-only memory).

> C: Fix atomic loads. [PR97981]
> 
> To handle atomic loads correctly, we need to move the code
> top qualifiers in lvalue conversion after
> the code that
> handles atomics.

The patch is OK.
diff mbox series

Patch

diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index cdc491a25fd..138af073925 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -2080,9 +2080,6 @@  convert_lvalue_to_rvalue (location_t loc, struct c_expr exp,
     exp = default_function_array_conversion (loc, exp);
   if (!VOID_TYPE_P (TREE_TYPE (exp.value)))
     exp.value = require_complete_type (loc, exp.value);
-  if (convert_p && !error_operand_p (exp.value)
-      && (TREE_CODE (TREE_TYPE (exp.value)) != ARRAY_TYPE))
-    exp.value = convert (build_qualified_type (TREE_TYPE (exp.value), TYPE_UNQUALIFIED),
exp.value);
   if (really_atomic_lvalue (exp.value))
     {
       vec<tree, va_gc> *params;
@@ -2119,6 +2116,9 @@  convert_lvalue_to_rvalue (location_t loc, struct c_expr exp,
       exp.value = build4 (TARGET_EXPR, nonatomic_type, tmp, func_call,
 			  NULL_TREE, NULL_TREE);
     }
+  if (convert_p && !error_operand_p (exp.value)
+      && (TREE_CODE (TREE_TYPE (exp.value)) != ARRAY_TYPE))
+    exp.value = convert (build_qualified_type (TREE_TYPE (exp.value), TYPE_UNQUALIFIED),
exp.value);
   return exp;
 }
 
diff --git a/gcc/testsuite/gcc.dg/pr60195.c b/gcc/testsuite/gcc.dg/pr60195.c
index 8eccf7f63ad..0a50a30be25 100644
--- a/gcc/testsuite/gcc.dg/pr60195.c
+++ b/gcc/testsuite/gcc.dg/pr60195.c
@@ -15,7 +15,7 @@  atomic_int
 fn2 (void)
 {
   atomic_int y = 0;
-  y;		/* { dg-warning "statement with no effect" } */
+  y;
   return y;
 }
 
diff --git a/gcc/testsuite/gcc.dg/pr97981.c b/gcc/testsuite/gcc.dg/pr97981.c
new file mode 100644
index 00000000000..846b8755c5b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr97981.c
@@ -0,0 +1,15 @@ 
+/* PR c/97981 */
+/* { dg-do compile } */
+/* { dg-options "-fdump-tree-original" } */
+/* { dg-final { scan-tree-dump-times "atomic_load" 2 "original" } } */
+
+
+void f(void)
+{
+	volatile _Atomic int x;
+	x;
+	volatile _Atomic double a;
+	double b;
+	b = a;
+}
+