From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1302 invoked by alias); 13 Jan 2015 14:56:13 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 1280 invoked by uid 89); 13 Jan 2015 14:56:12 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.8 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 13 Jan 2015 14:56:09 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t0DEu674021383 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Tue, 13 Jan 2015 09:56:06 -0500 Received: from [10.10.49.149] (vpn-49-149.rdu2.redhat.com [10.10.49.149]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t0DEu48q032330; Tue, 13 Jan 2015 09:56:04 -0500 Message-ID: <54B53203.6030304@redhat.com> Date: Tue, 13 Jan 2015 14:59:00 -0000 From: Andrew MacLeod User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 MIME-Version: 1.0 To: gcc-patches CC: Richard Biener , Jeff Law , Torvald Riegel , "Joseph S. Myers" , filter-gcc@preshing.com Subject: [PATCH] PR59448 - Promote consume to acquire Content-Type: multipart/mixed; boundary="------------030503020702070002090501" X-IsSubscribed: yes X-SW-Source: 2015-01/txt/msg00844.txt.bz2 This is a multi-part message in MIME format. --------------030503020702070002090501 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 1279 Lengthy discussion : https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59448 Basically we can generate incorrect code for an atomic consume operation in some circumstances. The general feeling seems to be that we should simply promote all consume operations to an acquire operation until there is a better definition/understanding of the consume model and how GCC can track it. I proposed a simple patch in the PR, and I have not seen or heard of any dissenting opinion. We should get this in before the end of stage 3 I think. The problem with the patch in the PR is the memory model is immediately promoted from consume to acquire. This happens *before* any of the memmodel checks are made. If a consume is illegally specified (such as in a compare_exchange), it gets promoted to acquire and the compiler doesn't report the error because it never sees the consume. This new patch simply makes the adjustment after any errors are checked on the originally specified model. It bootstraps on x86_64-unknown-linux-gnu and passes all regression testing. I also built an aarch64 compiler and it appears to issue the LDAR as specified in the PR, but anyone with a vested interest really ought to check it out with a real build to be sure. OK for trunk? Andrew --------------030503020702070002090501 Content-Type: text/x-patch; name="consume2.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="consume2.patch" Content-length: 3741 * builtins.c (memmodel_consume_fix) : New. Promote consume to acquire. (expand_builtin_atomic_exchange, expand_builtin_atomic_compare_exchange, expand_builtin_atomic_load, expand_builtin_atomic_fetch_op, expand_builtin_atomic_clear, expand_builtin_atomic_test_and_set, expand_builtin_atomic_thread_fence, expand_builtin_atomic_signal_fence): Call memmodel_consume_fix. Index: builtins.c =================================================================== *** builtins.c (revision 219462) --- builtins.c (working copy) *************** get_memmodel (tree exp) *** 5368,5373 **** --- 5368,5382 ---- return (enum memmodel) val; } + /* Workaround for Bugzilla 59448. GCC doesn't track consume properly, so + be conservative and promote consume to acquire. */ + static void + memmodel_consume_fix (enum memmodel &val) + { + if (val == MEMMODEL_CONSUME) + val = MEMMODEL_ACQUIRE; + } + /* Expand the __atomic_exchange intrinsic: TYPE __atomic_exchange (TYPE *object, TYPE desired, enum memmodel) EXP is the CALL_EXPR. *************** expand_builtin_atomic_exchange (machine_ *** 5389,5394 **** --- 5398,5405 ---- if (!flag_inline_atomics) return NULL_RTX; + memmodel_consume_fix (model); + /* Expand the operands. */ mem = get_builtin_sync_mem (CALL_EXPR_ARG (exp, 0), mode); val = expand_expr_force_mode (CALL_EXPR_ARG (exp, 1), mode); *************** expand_builtin_atomic_compare_exchange ( *** 5434,5439 **** --- 5445,5453 ---- if (!flag_inline_atomics) return NULL_RTX; + memmodel_consume_fix (success); + memmodel_consume_fix (failure); + /* Expand the operands. */ mem = get_builtin_sync_mem (CALL_EXPR_ARG (exp, 0), mode); *************** expand_builtin_atomic_load (machine_mode *** 5493,5498 **** --- 5507,5514 ---- if (!flag_inline_atomics) return NULL_RTX; + memmodel_consume_fix (model); + /* Expand the operand. */ mem = get_builtin_sync_mem (CALL_EXPR_ARG (exp, 0), mode); *************** expand_builtin_atomic_fetch_op (machine_ *** 5553,5558 **** --- 5569,5576 ---- model = get_memmodel (CALL_EXPR_ARG (exp, 2)); + memmodel_consume_fix (model); + /* Expand the operands. */ mem = get_builtin_sync_mem (CALL_EXPR_ARG (exp, 0), mode); val = expand_expr_force_mode (CALL_EXPR_ARG (exp, 1), mode); *************** expand_builtin_atomic_clear (tree exp) *** 5627,5632 **** --- 5645,5652 ---- return const0_rtx; } + memmodel_consume_fix (model); + if (HAVE_atomic_clear) { emit_insn (gen_atomic_clear (mem, model)); *************** expand_builtin_atomic_test_and_set (tree *** 5658,5664 **** mode = mode_for_size (BOOL_TYPE_SIZE, MODE_INT, 0); mem = get_builtin_sync_mem (CALL_EXPR_ARG (exp, 0), mode); model = get_memmodel (CALL_EXPR_ARG (exp, 1)); ! return expand_atomic_test_and_set (target, mem, model); } --- 5678,5684 ---- mode = mode_for_size (BOOL_TYPE_SIZE, MODE_INT, 0); mem = get_builtin_sync_mem (CALL_EXPR_ARG (exp, 0), mode); model = get_memmodel (CALL_EXPR_ARG (exp, 1)); ! memmodel_consume_fix (model); return expand_atomic_test_and_set (target, mem, model); } *************** static void *** 5797,5802 **** --- 5817,5823 ---- expand_builtin_atomic_thread_fence (tree exp) { enum memmodel model = get_memmodel (CALL_EXPR_ARG (exp, 0)); + memmodel_consume_fix (model); expand_mem_thread_fence (model); } *************** static void *** 5808,5813 **** --- 5829,5835 ---- expand_builtin_atomic_signal_fence (tree exp) { enum memmodel model = get_memmodel (CALL_EXPR_ARG (exp, 0)); + memmodel_consume_fix (model); expand_mem_signal_fence (model); } --------------030503020702070002090501--