From patchwork Mon May 2 12:52:28 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Biener X-Patchwork-Id: 617530 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]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3qz4376dhBz9t5C for ; Mon, 2 May 2016 22:52:51 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=ki5bKWY0; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; q=dns; s=default; b=o4rCUYoX29barBdPNc E6R7Q4/3W+j9WH0JdqWjhHI5OeMu4NEPdNOEFU74hKwXQxmzUKNcN+WDlGW5Z0d1 ZOD42gJYZnF8emNVKgz7UaL2arnFS2GZKbBpdwDFsl8gbM+1Sh9SbLB3o1VZxf0K LERJE/2E8mv60hxuk2WumlJpU= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; s=default; bh=4mLn2x80UU3JlMRInu4K00Qu SRI=; b=ki5bKWY0/ehLwIIti2vFGqd9jyClcU/+kHnr/qR/2xwEJ81xWxj9eZQ1 4zL53B3mjT1nMfiADgEFYqyRVfIwhxqG0xQmWAWXSXIXbzyk2G68FXbLbYDXW+sD q/nxKfdf7IJpCcrlsYe2/MOQlqEZOjYuvJGuyoCapDFO3Glq6k4= Received: (qmail 18276 invoked by alias); 2 May 2016 12:52:42 -0000 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 Received: (qmail 18262 invoked by uid 89); 2 May 2016 12:52:41 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy=favour, opinions, memcmp, translating X-HELO: mail-wm0-f54.google.com Received: from mail-wm0-f54.google.com (HELO mail-wm0-f54.google.com) (74.125.82.54) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Mon, 02 May 2016 12:52:31 +0000 Received: by mail-wm0-f54.google.com with SMTP id a17so140887193wme.0 for ; Mon, 02 May 2016 05:52:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc; bh=neg1Rxfg3flSwVG3LkSgq11H0VOCjpUPw64oAMcRPcs=; b=dY4KxQfQmaSu/ygilc3Jn05oP21qrGSaXG8XV+1nhtXFk9HxWFuzerv2Hz6OAN5IvT yGLDOVAR1TkCs4DdqdEEvNSa+Xa2hZADa6Mu/dIKQ7GXTTHBQGrFuIy7OzBuvIPyGZ4L dY5d2VqJQIXba06l46j1DxNLUDwFKKcbxuBWMpBKDn9t1XG5wkahnJJurCnOXWbFjZWV nd4DzsWelOktBxKhZd2reW9Al+51kaJkFOQ5QOks0Ffo+vxApiswGLeYMtjbVS9BAy79 N35KKFWO6vD+5ttea6lvB65HB27WqUFEpSJKdPfb3rQ8O+RsSU2VTpYuJ8qLbtyy79wj ypkg== X-Gm-Message-State: AOPr4FUk71d2s4LyFvEmc07CxNrMJ2ARVbogPy28o0E5Nlf80hLwrGX/4HoXXCwoOqpjh8etTeD5CUSIQhjAnQ== MIME-Version: 1.0 X-Received: by 10.194.223.70 with SMTP id qs6mr35843543wjc.119.1462193548809; Mon, 02 May 2016 05:52:28 -0700 (PDT) Received: by 10.194.113.102 with HTTP; Mon, 2 May 2016 05:52:28 -0700 (PDT) In-Reply-To: <5722581B.5050402@redhat.com> References: <56992541.3090402@redhat.com> <5722581B.5050402@redhat.com> Date: Mon, 2 May 2016 14:52:28 +0200 Message-ID: Subject: Re: Thoughts on memcmp expansion (PR43052) From: Richard Biener To: Bernd Schmidt Cc: GCC Patches , Nick Clifton X-IsSubscribed: yes On Thu, Apr 28, 2016 at 8:36 PM, Bernd Schmidt wrote: > On 01/18/2016 10:22 AM, Richard Biener wrote: >> >> See also https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52171 - the >> inline expansion >> for small sizes and equality compares should be done on GIMPLE. Today the >> strlen pass might be an appropriate place to do this given its >> superior knowledge >> about string lengths. >> >> The idea of turning eq feeding memcmp into a special memcmp_eq is good but >> you have to avoid doing that too early - otherwise you'd lose on >> >> res = memcmp (p, q, sz); >> if (memcmp (p, q, sz) == 0) >> ... >> >> that is, you have to make sure CSE got the chance to common the two calls. >> This is why I think this kind of transform needs to happen in specific >> places >> (like during strlen opt) rather than in generic folding. > > > Ok, here's an update. I kept pieces of your patch from that PR, but also > translating memcmps larger than a single operation into memcmp_eq as in my > previous patch. > > Then, I added by_pieces infrastructure for memcmp expansion. To avoid any > more code duplication in this area, I abstracted the existing code and > converted it to C++ classes since that seemed to fit pretty well. > > There are a few possible ways I could go with this, which is why I'm posting > it more as a RFD at this point. > - should store_by_pieces be eliminated in favour of doing just > move_by_pieces with constfns? > - the C++ification could be extended, making move_by_pieces_d and > compare_by_pieces_d classes inheriting from a common base. This > would get rid of the callbacks, replacing them with virtuals, > and also make some of the current struct members private. > - could move all of the by_pieces stuff out into a separate file? > > Later, I think we'll also want to extend this to allow vector mode > operations, but I think that's a separate patch. > > So, opinions what I should be doing with this patch? FWIW it bootstraps and > tests OK on x86_64-linux. +struct pieces_addr +{ ... + void *m_cfndata; +public: + pieces_addr (rtx, bool, by_pieces_constfn, void *); unless you strick private: somewhere the public: is redundant Index: gcc/tree-ssa-forwprop.c =================================================================== --- gcc/tree-ssa-forwprop.c (revision 235474) +++ gcc/tree-ssa-forwprop.c (working copy) @@ -1435,6 +1435,76 @@ simplify_builtin_call (gimple_stmt_itera } } break; + + case BUILT_IN_MEMCMP: + { I think this doesn't belong in forwprop. If we want to stick it into a pass rather than folding it should be in tree-ssa-strlen.c. + if (tree_fits_uhwi_p (len) + && (leni = tree_to_uhwi (len)) <= GET_MODE_SIZE (word_mode) + && exact_log2 (leni) != -1 + && (align1 = get_pointer_alignment (arg1)) >= leni * BITS_PER_UNIT + && (align2 = get_pointer_alignment (arg2)) >= leni * BITS_PER_UNIT) + { + location_t loc = gimple_location (stmt2); + tree type, off; + type = build_nonstandard_integer_type (leni * BITS_PER_UNIT, 1); + gcc_assert (GET_MODE_SIZE (TYPE_MODE (type)) == leni); + off = build_int_cst + (build_pointer_type_for_mode (char_type_node, ptr_mode, true), 0); + arg1 = build2_loc (loc, MEM_REF, type, arg1, off); + arg2 = build2_loc (loc, MEM_REF, type, arg2, off); + res = fold_convert_loc (loc, TREE_TYPE (res), + fold_build2_loc (loc, NE_EXPR, + boolean_type_node, + arg1, arg2)); + gimplify_and_update_call_from_tree (gsi_p, res); + return true; + } for this part see gimple_fold_builtin_memory_op handling of /* If we can perform the copy efficiently with first doing all loads and then all stores inline it that way. Currently efficiently means that we can load all the memory into a single integer register which is what MOVE_MAX gives us. */ we might want to share a helper yielding the type of the load/store or NULL if not possible. Note that we can handle size-1 memcmp even for ordered compares. Jakub, where do you think this fits best? Note that gimple-fold.c may not use immediate uses but would have to match this from the comparison (I still have to find a way to handle this in match.pd where the result expression contains virtual operands in the not toplevel stmt). Richard. > > Bernd