Patchwork [C++] Fix __atomic_exchange (PR c++/60689)

login
register
mail settings
Submitter Jakub Jelinek
Date March 28, 2014, 10:47 a.m.
Message ID <20140328104752.GJ1817@tucnak.redhat.com>
Download mbox | patch
Permalink /patch/334654/
State New
Headers show

Comments

Jakub Jelinek - March 28, 2014, 10:47 a.m.
Hi!

__atomic_exchange doesn't work in C++.  The problem is that
add_atomic_size_parameter, if there is no room in params vector, creates
new params vector big enough that the extra argument fixs in, but doesn't
add the extra argument in, because it relies on the subsequent
build_function_call_vec to call resolve_overloaded_builtin recursively and
add the parameter.  The C build_function_call_vec does that, but C++
doesn't, there resolve_overloaded_builtin is called from finish_call_expr
instead.

My first attempt to fix this - changing add_atomic_size_parameter to add
that argument - broke C, where the recursive resolve_overloaded_builtin ->
get_atomic_generic_size would complain about too many arguments.

Here is one possible fix for this, let the C++ build_function_call_vec
(which is only called from two c-family/c-common.c spots that expect this
behavior) behave more like the C call.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk (and
4.8)?

Another alternative is some static flag in resolve_overloaded_function that
would just punt if the function is called recursively, but I think if we can
avoid adding global state, we should (otherwise JIT won't like it too much).

Yet another possibility would be to rename all calls in C FE to
build_function_call_vec to say c_build_function_call_vec and add that
function which would call resolve_overloaded_builtin and then tail call to
build_function_call_vec which wouldn't do that.  Then c-family/ would keep
its current two calls to that function, which wouldn't recurse anymore, and
we'd need to change add_atomic_size_parameter to push the argument.

2014-03-28  Jakub Jelinek  <jakub@redhat.com>

	PR c++/60689
	* typeck.c (build_function_call_vec): Call resolve_overloaded_builtin.

	* c-c++-common/pr60689.c: New test.


	Jakub
Jason Merrill - March 28, 2014, 5:46 p.m.
On 03/28/2014 06:47 AM, Jakub Jelinek wrote:
> 	* typeck.c (build_function_call_vec): Call resolve_overloaded_builtin.

I expect this will break in templates if arguments are dependent.

Jason
Jakub Jelinek - March 28, 2014, 5:51 p.m.
On Fri, Mar 28, 2014 at 01:46:09PM -0400, Jason Merrill wrote:
> On 03/28/2014 06:47 AM, Jakub Jelinek wrote:
> >	* typeck.c (build_function_call_vec): Call resolve_overloaded_builtin.
> 
> I expect this will break in templates if arguments are dependent.

The only problem with this patch is potentially ObjC, I've missed that it
also calls build_function_call_vec; in c-family and cp/ proper
build_function_call_vec is only called from within
resolve_overloaded_builtin itself, thus it shouldn't see dependent args.

	Jakub

Patch

--- gcc/cp/typeck.c.jj	2014-03-10 10:50:14.000000000 +0100
+++ gcc/cp/typeck.c	2014-03-28 08:07:49.737656541 +0100
@@ -3363,10 +3363,23 @@  build_function_call (location_t /*loc*/,
 
 /* Used by the C-common bits.  */
 tree
-build_function_call_vec (location_t /*loc*/, vec<location_t> /*arg_loc*/,
+build_function_call_vec (location_t loc, vec<location_t> /*arg_loc*/,
 			 tree function, vec<tree, va_gc> *params,
 			 vec<tree, va_gc> * /*origtypes*/)
 {
+  /* This call is here to match what the C FE does in its
+     build_function_call_vec.  See PR60689.  */
+  if (TREE_CODE (function) == FUNCTION_DECL)
+    {
+      /* Implement type-directed function overloading for builtins.
+	 resolve_overloaded_builtin and targetm.resolve_overloaded_builtin
+	 handle all the type checking.  The result is a complete expression
+	 that implements this function call.  */
+      tree tem = resolve_overloaded_builtin (loc, function, params);
+      if (tem)
+	return tem;
+    }
+
   vec<tree, va_gc> *orig_params = params;
   tree ret = cp_build_function_call_vec (function, &params,
 					 tf_warning_or_error);
--- gcc/testsuite/c-c++-common/pr60689.c.jj	2014-03-27 22:06:31.703103613 +0100
+++ gcc/testsuite/c-c++-common/pr60689.c	2014-03-27 22:06:46.542024952 +0100
@@ -0,0 +1,10 @@ 
+/* PR c++/60689 */
+/* { dg-do compile } */
+
+struct S { char x[9]; };
+
+void
+foo (struct S *x, struct S *y, struct S *z)
+{
+  __atomic_exchange (x, y, z, __ATOMIC_SEQ_CST);
+}