From patchwork Wed Aug 10 10:26:39 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Biener X-Patchwork-Id: 109357 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 77B10B71DF for ; Wed, 10 Aug 2011 20:27:04 +1000 (EST) Received: (qmail 20134 invoked by alias); 10 Aug 2011 10:27:02 -0000 Received: (qmail 20123 invoked by uid 22791); 10 Aug 2011 10:27:01 -0000 X-SWARE-Spam-Status: No, hits=-3.7 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, TW_CP X-Spam-Check-By: sourceware.org Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 10 Aug 2011 10:26:41 +0000 Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.221.2]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx2.suse.de (Postfix) with ESMTP id A5B8F89471; Wed, 10 Aug 2011 12:26:39 +0200 (CEST) Date: Wed, 10 Aug 2011 12:26:39 +0200 (CEST) From: Richard Guenther To: gcc-patches@gcc.gnu.org Cc: jason@redhat.com Subject: [PATCH][C++] Remove last use of can_trust_pointer_alignment Message-ID: User-Agent: Alpine 2.00 (LNX 1167 2008-08-23) MIME-Version: 1.0 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Nothing in the middle-end happens conditional on can_trust_pointer_alignment anymore - we can always "trust" pointer alignment, that function and its comment is somewhat gross. In fact we can now track alignment properly via CCP and thus the hack in cfgexpand.c:expand_call_stmt should not be neccessary anymore. Now, looking at the use of can_trust_pointer_alignment in build_over_call and especially its comment: if (!can_trust_pointer_alignment ()) { /* If we can't be sure about pointer alignment, a call to __builtin_memcpy is expanded as a call to memcpy, which is invalid with identical args. Otherwise it is expanded as a block move, which should be safe. */ arg0 = save_expr (arg0); arg1 = save_expr (arg1); test = build2 (EQ_EXPR, boolean_type_node, arg0, arg1); } "Otherwise it is expanded as a block move" is certainly not true. Whether it is expanded as block move depends on optimization setting, target costs and, well, a way to do block moves (we fall back to memcpy after all). So the patch changes the above to if (!optimize), but I'd argue we can as well either remove the conditional or emit it unconditional. Dependent on whether we believe a memcpy implementation exists in the wild that asserts that src != dst. Independent of the can_trust_pointer_alignment issue nowadays the case of !tree_int_cst_equal (TYPE_SIZE (type), TYPE_SIZE (as_base)) should use a MODIFY_EXPR with two MEM_REF operands and a proper alias set, which would avoid the address-taking (and thus aliasing) because of the memcpy and also would avoid forcing alias-set zero because of the memcpy. Thus (untested), something like Index: gcc/cp/call.c =================================================================== --- gcc/cp/call.c (revision 177597) +++ gcc/cp/call.c (working copy) @@ -6379,35 +6379,21 @@ build_over_call (struct z_candidate *can } else { - /* We must only copy the non-tail padding parts. - Use __builtin_memcpy for the bitwise copy. - FIXME fix 22488 so we can go back to using MODIFY_EXPR - instead of an explicit call to memcpy. */ - - tree arg0, arg1, arg2, t; + /* We must only copy the non-tail padding parts. */ + tree arg0, arg2, t; tree test = NULL_TREE; arg2 = TYPE_SIZE_UNIT (as_base); - arg1 = arg; arg0 = cp_build_addr_expr (to, complain); - if (!can_trust_pointer_alignment ()) - { - /* If we can't be sure about pointer alignment, a call - to __builtin_memcpy is expanded as a call to memcpy, which - is invalid with identical args. Otherwise it is - expanded as a block move, which should be safe. */ - arg0 = save_expr (arg0); - arg1 = save_expr (arg1); - test = build2 (EQ_EXPR, boolean_type_node, arg0, arg1); - } - t = implicit_built_in_decls[BUILT_IN_MEMCPY]; - t = build_call_n (t, 3, arg0, arg1, arg2); - - t = convert (TREE_TYPE (arg0), t); - if (test) - t = build3 (COND_EXPR, TREE_TYPE (t), test, arg0, t); - val = cp_build_indirect_ref (t, RO_NULL, complain); + t = build_array_type (char_type_node, build_index_type (arg2)); + t = build2 (MODIFY_EXPR, void_type_node, + build2 (MEM_REF, t, to, + build_int_cst (build_pointer_type (type), 0)), + build2 (MEM_REF, t, arg, + build_int_cst (build_pointer_type (type), 0))); + val = build2 (COMPOUND_EXPR, TREE_TYPE (to), + t, to); TREE_NO_WARNING (val) = 1; } which does a block copy of size arg2 (by using a char[arg2] array a type for the mem-ref) using the alias-set of type (by specifying the offset of the mem-ref with using a type of type *). Well, but I'm bootstrapping and testing the following now, on x86_64-unknown-linux-gnu (because I want to get rid of that can_trust_pointer_alignment function). Ok for trunk? Thanks, Richard. 2011-08-10 Richard Guenther cp/ * call.c (build_over_call): Check for optimize instead of can_trust_pointer_alignment. Adjust comment. Index: gcc/cp/call.c =================================================================== --- gcc/cp/call.c (revision 177613) +++ gcc/cp/call.c (working copy) @@ -6778,12 +6778,14 @@ build_over_call (struct z_candidate *can arg1 = arg; arg0 = cp_build_addr_expr (to, complain); - if (!can_trust_pointer_alignment ()) + if (!optimize) { - /* If we can't be sure about pointer alignment, a call - to __builtin_memcpy is expanded as a call to memcpy, which - is invalid with identical args. Otherwise it is - expanded as a block move, which should be safe. */ + /* If we are not optimizing, a call to __builtin_memcpy is + expanded as a call to memcpy, which is invalid with identical + args. Otherwise it is expanded as a block move, which should + be safe. */ + /* ??? It is of course only sometimes expanded as a block + move. */ arg0 = save_expr (arg0); arg1 = save_expr (arg1); test = build2 (EQ_EXPR, boolean_type_node, arg0, arg1);