From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 92218 invoked by alias); 7 Sep 2016 20:41:00 -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 92193 invoked by uid 89); 7 Sep 2016 20:40:59 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=2.5 required=5.0 tests=BAYES_50,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=no version=3.3.2 spammy=hash_set, optsc, opts.c, UD:opts.c X-HELO: mail-wm0-f49.google.com Received: from mail-wm0-f49.google.com (HELO mail-wm0-f49.google.com) (74.125.82.49) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 07 Sep 2016 20:40:49 +0000 Received: by mail-wm0-f49.google.com with SMTP id w12so1782569wmf.0 for ; Wed, 07 Sep 2016 13:40:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:user-agent:in-reply-to:references:mime-version :content-transfer-encoding:subject:from:date:to:cc:message-id; bh=7TdtIbVc2GUxHYpdXLsIuKohOuCoLG8ZePuGIqwUES8=; b=OQimP/U2NQ6haWE9gOWra/GwXfMXkXOSRvQk7jvuOlKa7tf5wIb9QmuKqXLF/4SEKW J17M8k3DM1Q3Vpz8dZS8Yem/P5oEHud3V3nvBdPgFWDToG7sZwJYj5h71OUHT8ybTZVz qAoASkEdycmaNr5QqS8tawt9rSTQcj50Mlz5m1gG/VFyB6whD81qxR+48RJVRpR5T8SV /tXTrL4W92rmS5uPbkgL80W17kZXOhSUjP2IxY6iefR92YHjkq+wKrE75rwN1pKO+Y1J VjZhffQIg9bW8ZK0CF78rFB4LkzkSY8scwd52oP9gNDFMR3JpeOKXvYXFe2a7SGU7JW6 XPPA== X-Gm-Message-State: AE9vXwOZAWs6HLoECOGSTmGCVqFd1chc10Zb1CjkMAS3rW6aRcFmXkLTFPxJZt48loe2pA== X-Received: by 10.194.140.201 with SMTP id ri9mr25451616wjb.195.1473280847046; Wed, 07 Sep 2016 13:40:47 -0700 (PDT) Received: from [192.168.100.51] (91-119-139-156.dsl.dynamic.surfer.at. [91.119.139.156]) by smtp.gmail.com with ESMTPSA id w203sm5972239wmw.7.2016.09.07.13.40.46 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 07 Sep 2016 13:40:46 -0700 (PDT) User-Agent: K-9 Mail for Android In-Reply-To: <57CEDD67.6010801@foss.arm.com> References: <57CEDD67.6010801@foss.arm.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset=UTF-8 Subject: Re: [PATCH][v3] GIMPLE store merging pass From: Bernhard Reutner-Fischer Date: Wed, 07 Sep 2016 20:44:00 -0000 To: Kyrill Tkachov ,GCC Patches CC: Richard Biener Message-ID: X-IsSubscribed: yes X-SW-Source: 2016-09/txt/msg00408.txt.bz2 On September 6, 2016 5:14:47 PM GMT+02:00, Kyrill Tkachov wrote: >Hi all, s/contigous/contiguous/ s/ where where/ where/ +struct merged_store_group +{ + HOST_WIDE_INT start; + HOST_WIDE_INT width; + unsigned char *val; + unsigned int align; + auto_vec stores; + /* We record the first and last original statements in the sequence because + because we'll need their vuse/vdef and replacement position. */ + gimple *last_stmt; s/ because because/ because/ Why aren't these two HWIs unsigned, likewise in store_immediate_info and in most other spots in the patch? + fprintf (dump_file, "Afer writing "); s/Afer /After/ /access if prohibitively slow/s/ if /is / I'd get rid of successful_p in imm_store_chain_info::output_merged_stores. +unsigned int +pass_store_merging::execute (function *fun) +{ + basic_block bb; + hash_set orig_stmts; + + FOR_EACH_BB_FN (bb, fun) + { + gimple_stmt_iterator gsi; + HOST_WIDE_INT num_statements = 0; + /* Record the original statements so that we can keep track of + statements emitted in this pass and not re-process new + statements. */ + for (gsi = gsi_after_labels (bb); !gsi_end_p (gsi); gsi_next (&gsi)) + { + gimple_set_visited (gsi_stmt (gsi), false); + num_statements++; + } + + if (num_statements < 2) + continue; What about debug statements? ISTM you should skip those. (Isn't visited reset before entry of a pass?) Maybe I missed the bikeshedding about the name but I'd have used -fmerge-stores instead. Thanks, > >The v3 of this patch addresses feedback I received on the version >posted at [1]. >The merged store buffer is now represented as a char array that we >splat values onto with >native_encode_expr and native_interpret_expr. This allows us to merge >anything that native_encode_expr >accepts, including floating point values and short vectors. So this >version extends the functionality >of the previous one in that it handles floating point values as well. > >The first phase of the algorithm that detects the contiguous stores is >also slightly refactored according >to feedback to read more fluently. > >Richi, I experimented with merging up to MOVE_MAX bytes rather than >word size but I got worse results on aarch64. >MOVE_MAX there is 16 (because it has load/store register pair >instructions) but the 128-bit immediates that we ended >synthesising were too complex. Perhaps the TImode immediate store RTL >expansions could be improved, but for now >I've left the maximum merge size to be BITS_PER_WORD. > >I've disabled the pass for PDP-endian targets as the merging code >proved to be quite fiddly to get right for different >endiannesses and I didn't feel comfortable writing logic for >BYTES_BIG_ENDIAN != WORDS_BIG_ENDIAN targets without serious >testing capabilities. I hope that's ok (I note the bswap pass also >doesn't try to do anything on such targets). > >Tested on arm, aarch64, x86_64 and on big-endian arm and aarch64. > >How does this version look? >Thanks, >Kyrill > >[1] https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01512.html > >2016-09-06 Kyrylo Tkachov > > PR middle-end/22141 > * Makefile.in (OBJS): Add gimple-ssa-store-merging.o. > * common.opt (fstore-merging): New Optimization option. > * opts.c (default_options_table): Add entry for > OPT_ftree_store_merging. > * params.def (PARAM_STORE_MERGING_ALLOW_UNALIGNED): Define. > * passes.def: Insert pass_tree_store_merging. > * tree-pass.h (make_pass_store_merging): Declare extern > prototype. > * gimple-ssa-store-merging.c: New file. > * doc/invoke.texi (Optimization Options): Document > -fstore-merging. > >2016-09-06 Kyrylo Tkachov > Jakub Jelinek > > PR middle-end/22141 > * gcc.c-torture/execute/pr22141-1.c: New test. > * gcc.c-torture/execute/pr22141-2.c: Likewise. > * gcc.target/aarch64/ldp_stp_1.c: Adjust for -fstore-merging. > * gcc.target/aarch64/ldp_stp_4.c: Likewise. > * gcc.dg/store_merging_1.c: New test. > * gcc.dg/store_merging_2.c: Likewise. > * gcc.dg/store_merging_3.c: Likewise. > * gcc.dg/store_merging_4.c: Likewise. > * gcc.dg/store_merging_5.c: Likewise. > * gcc.dg/store_merging_6.c: Likewise. > * gcc.target/i386/pr22141.c: Likewise. > * gcc.target/i386/pr34012.c: Add -fno-store-merging to dg-options.