From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 78824 invoked by alias); 20 Dec 2017 12:59:26 -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 78808 invoked by uid 89); 20 Dec 2017 12:59:25 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.0 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,GIT_PATCH_3,KAM_ASCII_DIVIDERS,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=nn, 036, revamp X-HELO: mail-wr0-f172.google.com Received: from mail-wr0-f172.google.com (HELO mail-wr0-f172.google.com) (209.85.128.172) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 20 Dec 2017 12:59:22 +0000 Received: by mail-wr0-f172.google.com with SMTP id u19so15831324wrc.3 for ; Wed, 20 Dec 2017 04:59:21 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:mail-followup-to:cc:subject:references :date:in-reply-to:message-id:user-agent:mime-version; bh=dlep9D8gY7VTEvfT1unbL833wbdcn3sVSHZ77j0Cg0w=; b=HYImk/QcywWBRTd5Ek6WkYjfGDS90u60Tp+rnyPnUo6Gk9dLDYvXRg241y4sdZzTlv Bk7aVGzhW/jXCmGv7yOKJC3nlWjLKgRXDA0l9zR231SyCsoWqF6VOMC/0VwI+uDetmfF tSC1T/93pmyaJvJC9suyi9thmSD+zI1r4qswyQ2scMjP/I5VuNrFnu6vo9tfWiNE9OCT bw4Vmkf/FL1TBPs8QYRXQLrbZUz6h3bEV+R9QEsOTTWjdQUsHmVpEJ4bnbNeXIqhO4tf HfivClru7FzTpC7o1CNljooXdRrM7XiHRs3OBib2/K0vJS0RpUPraUEy5aaKk+ZYyNx6 8vtQ== X-Gm-Message-State: AKGB3mLhvqP7mWACYIzOR1rjg8MV5ugJnu9TDSPuaV0Yg+6NgxWWy6cx 4qo2/fEzZNXhsYjsd08JeNtct4XySOc= X-Google-Smtp-Source: ACJfBouXcUEVdNXFa0xF0jp7w93aroAZZmgoEyNX34YNP+c2qmDkAw7I2VyT6thXReU1iYUE8LPfeg== X-Received: by 10.223.176.242 with SMTP id j47mr8655538wra.178.1513774759758; Wed, 20 Dec 2017 04:59:19 -0800 (PST) Received: from localhost ([2.25.234.26]) by smtp.gmail.com with ESMTPSA id x52sm20327887wrb.25.2017.12.20.04.59.18 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 20 Dec 2017 04:59:19 -0800 (PST) From: Richard Sandiford To: Jeff Law Mail-Followup-To: Jeff Law ,gcc-patches@gcc.gnu.org, richard.sandiford@linaro.org Cc: gcc-patches@gcc.gnu.org Subject: Re: [039/nnn] poly_int: pass_store_merging::execute References: <871sltvm7r.fsf@linaro.org> <877evlokbw.fsf@linaro.org> <276f5c34-29c2-0d29-034c-d602f2eb2109@redhat.com> Date: Wed, 20 Dec 2017 12:59:00 -0000 In-Reply-To: <276f5c34-29c2-0d29-034c-d602f2eb2109@redhat.com> (Jeff Law's message of "Tue, 28 Nov 2017 10:51:54 -0700") Message-ID: <87d139368q.fsf@linaro.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2017-12/txt/msg01352.txt.bz2 Jeff Law writes: > On 10/23/2017 11:17 AM, Richard Sandiford wrote: >> This patch makes pass_store_merging::execute track polynomial sizes >> and offsets. >> >> >> 2017-10-23 Richard Sandiford >> Alan Hayward >> David Sherwood >> >> gcc/ >> * gimple-ssa-store-merging.c (pass_store_merging::execute): Track >> polynomial sizes and offsets. > OK. THough I wouldn't be surprised if this needs revamping after > Jakub's work in this space. Yeah, it needed quite a big revamp in the end. Same idea, just in different places. And... > It wasn't clear why you moved some of the code which computes invalid vs > where we test invalid, but I don't see any problem with that movement of > code. ...this part fortunately went away with the new code structure. Here's what I applied after retesting. It now fits in the series after [036/nnn]. Thanks, Richard 2017-12-20 Richard Sandiford Alan Hayward David Sherwood gcc/ * poly-int-types.h (round_down_to_byte_boundary): New macro. (round_up_to_byte_boundary): Likewise. * expr.h (get_bit_range): Add temporary shim. * gimple-ssa-store-merging.c (store_operand_info): Change the bitsize, bitpos, bitregion_start and bitregion_end fields from unsigned HOST_WIDE_INT to poly_uint64. (merged_store_group): Likewise load_align_base. (compatible_load_p, compatible_load_p): Update accordingly. (imm_store_chain_info::coalesce_immediate_stores): Likewise. (split_group, imm_store_chain_info::output_merged_store): Likewise. (mem_valid_for_store_merging): Return the bitsize, bitpos, bitregion_start and bitregion_end as poly_uint64s rather than unsigned HOST_WIDE_INTs. Track polynomial offsets internally. (handled_load): Take the bitsize, bitpos, bitregion_start and bitregion_end as poly_uint64s rather than unsigned HOST_WIDE_INTs. (pass_store_merging::process_store): Update call to mem_valid_for_store_merging. Index: gcc/poly-int-types.h =================================================================== --- gcc/poly-int-types.h 2017-12-20 09:36:17.817094740 +0000 +++ gcc/poly-int-types.h 2017-12-20 09:37:07.445382807 +0000 @@ -60,6 +60,18 @@ #define bits_to_bytes_round_up(X) force_ of bytes in size. */ #define num_trailing_bits(X) force_get_misalignment (X, BITS_PER_UNIT) +/* Round bit quantity X down to the nearest byte boundary. + + This is safe because non-constant mode sizes must be a whole number + of bytes in size. */ +#define round_down_to_byte_boundary(X) force_align_down (X, BITS_PER_UNIT) + +/* Round bit quantity X up the nearest byte boundary. + + This is safe because non-constant mode sizes must be a whole number + of bytes in size. */ +#define round_up_to_byte_boundary(X) force_align_up (X, BITS_PER_UNIT) + /* Return the size of an element in a vector of size SIZE, given that the vector has NELTS elements. The return value is in the same units as SIZE (either bits or bytes). Index: gcc/expr.h =================================================================== --- gcc/expr.h 2017-12-20 09:36:17.817094740 +0000 +++ gcc/expr.h 2017-12-20 09:37:07.444382841 +0000 @@ -243,6 +243,15 @@ extern bool emit_push_insn (rtx, machine extern void get_bit_range (unsigned HOST_WIDE_INT *, unsigned HOST_WIDE_INT *, tree, HOST_WIDE_INT *, tree *); +/* Temporary. */ +inline void +get_bit_range (poly_uint64_pod *bitstart, poly_uint64_pod *bitend, tree exp, + poly_int64_pod *bitpos, tree *offset) +{ + get_bit_range (&bitstart->coeffs[0], &bitend->coeffs[0], exp, + &bitpos->coeffs[0], offset); +} + /* Expand an assignment that stores the value of FROM into TO. */ extern void expand_assignment (tree, tree, bool); Index: gcc/gimple-ssa-store-merging.c =================================================================== --- gcc/gimple-ssa-store-merging.c 2017-12-20 09:36:17.817094740 +0000 +++ gcc/gimple-ssa-store-merging.c 2017-12-20 09:37:07.445382807 +0000 @@ -1321,10 +1321,10 @@ struct store_operand_info { tree val; tree base_addr; - unsigned HOST_WIDE_INT bitsize; - unsigned HOST_WIDE_INT bitpos; - unsigned HOST_WIDE_INT bitregion_start; - unsigned HOST_WIDE_INT bitregion_end; + poly_uint64 bitsize; + poly_uint64 bitpos; + poly_uint64 bitregion_start; + poly_uint64 bitregion_end; gimple *stmt; bool bit_not_p; store_operand_info (); @@ -1414,7 +1414,7 @@ struct merged_store_group /* The size of the allocated memory for val and mask. */ unsigned HOST_WIDE_INT buf_size; unsigned HOST_WIDE_INT align_base; - unsigned HOST_WIDE_INT load_align_base[2]; + poly_uint64 load_align_base[2]; unsigned int align; unsigned int load_align[2]; @@ -2198,8 +2198,8 @@ compatible_load_p (merged_store_group *m { store_immediate_info *infof = merged_store->stores[0]; if (!info->ops[idx].base_addr - || (info->ops[idx].bitpos - infof->ops[idx].bitpos - != info->bitpos - infof->bitpos) + || maybe_ne (info->ops[idx].bitpos - infof->ops[idx].bitpos, + info->bitpos - infof->bitpos) || !operand_equal_p (info->ops[idx].base_addr, infof->ops[idx].base_addr, 0)) return false; @@ -2229,7 +2229,7 @@ compatible_load_p (merged_store_group *m the construction of the immediate chain info guarantees no intervening stores, so no further checks are needed. Example: _1 = s.a; _2 = _1 & -7; s.a = _2; _3 = s.b; _4 = _3 & -7; s.b = _4; */ - if (info->ops[idx].bitpos == info->bitpos + if (known_eq (info->ops[idx].bitpos, info->bitpos) && operand_equal_p (info->ops[idx].base_addr, base_addr, 0)) return true; @@ -2624,8 +2624,8 @@ imm_store_chain_info::coalesce_immediate && infof->ops[1].base_addr && info->ops[0].base_addr && info->ops[1].base_addr - && (info->ops[1].bitpos - infof->ops[0].bitpos - == info->bitpos - infof->bitpos) + && known_eq (info->ops[1].bitpos - infof->ops[0].bitpos, + info->bitpos - infof->bitpos) && operand_equal_p (info->ops[1].base_addr, infof->ops[0].base_addr, 0)) { @@ -3031,11 +3031,12 @@ split_group (merged_store_group *group, for (int i = 0; i < 2; ++i) if (group->load_align[i]) { - align_bitpos = try_bitpos - group->stores[0]->bitpos; - align_bitpos += group->stores[0]->ops[i].bitpos; - align_bitpos -= group->load_align_base[i]; - align_bitpos &= (group_load_align - 1); - if (align_bitpos) + align_bitpos + = known_alignment (try_bitpos + - group->stores[0]->bitpos + + group->stores[0]->ops[i].bitpos + - group->load_align_base[i]); + if (align_bitpos & (group_load_align - 1)) { unsigned HOST_WIDE_INT a = least_bit_hwi (align_bitpos); load_align = MIN (load_align, a); @@ -3491,10 +3492,10 @@ imm_store_chain_info::output_merged_stor unsigned HOST_WIDE_INT load_align = group->load_align[j]; unsigned HOST_WIDE_INT align_bitpos - = (try_pos * BITS_PER_UNIT - - split_store->orig_stores[0]->bitpos - + op.bitpos) & (load_align - 1); - if (align_bitpos) + = known_alignment (try_pos * BITS_PER_UNIT + - split_store->orig_stores[0]->bitpos + + op.bitpos); + if (align_bitpos & (load_align - 1)) load_align = least_bit_hwi (align_bitpos); tree load_int_type @@ -3502,10 +3503,11 @@ imm_store_chain_info::output_merged_stor load_int_type = build_aligned_type (load_int_type, load_align); - unsigned HOST_WIDE_INT load_pos - = (try_pos * BITS_PER_UNIT - - split_store->orig_stores[0]->bitpos - + op.bitpos) / BITS_PER_UNIT; + poly_uint64 load_pos + = exact_div (try_pos * BITS_PER_UNIT + - split_store->orig_stores[0]->bitpos + + op.bitpos, + BITS_PER_UNIT); ops[j] = fold_build2 (MEM_REF, load_int_type, load_addr[j], build_int_cst (offset_type, load_pos)); if (TREE_CODE (ops[j]) == MEM_REF) @@ -3811,30 +3813,28 @@ rhs_valid_for_store_merging_p (tree rhs) case. */ static tree -mem_valid_for_store_merging (tree mem, unsigned HOST_WIDE_INT *pbitsize, - unsigned HOST_WIDE_INT *pbitpos, - unsigned HOST_WIDE_INT *pbitregion_start, - unsigned HOST_WIDE_INT *pbitregion_end) -{ - HOST_WIDE_INT bitsize; - HOST_WIDE_INT bitpos; - unsigned HOST_WIDE_INT bitregion_start = 0; - unsigned HOST_WIDE_INT bitregion_end = 0; +mem_valid_for_store_merging (tree mem, poly_uint64 *pbitsize, + poly_uint64 *pbitpos, + poly_uint64 *pbitregion_start, + poly_uint64 *pbitregion_end) +{ + poly_int64 bitsize, bitpos; + poly_uint64 bitregion_start = 0, bitregion_end = 0; machine_mode mode; int unsignedp = 0, reversep = 0, volatilep = 0; tree offset; tree base_addr = get_inner_reference (mem, &bitsize, &bitpos, &offset, &mode, &unsignedp, &reversep, &volatilep); *pbitsize = bitsize; - if (bitsize == 0) + if (known_eq (bitsize, 0)) return NULL_TREE; if (TREE_CODE (mem) == COMPONENT_REF && DECL_BIT_FIELD_TYPE (TREE_OPERAND (mem, 1))) { get_bit_range (&bitregion_start, &bitregion_end, mem, &bitpos, &offset); - if (bitregion_end) - ++bitregion_end; + if (maybe_ne (bitregion_end, 0U)) + bitregion_end += 1; } if (reversep) @@ -3850,24 +3850,20 @@ mem_valid_for_store_merging (tree mem, u PR 23684 and this way we can catch more chains. */ else if (TREE_CODE (base_addr) == MEM_REF) { - offset_int bit_off, byte_off = mem_ref_offset (base_addr); - bit_off = byte_off << LOG2_BITS_PER_UNIT; + poly_offset_int byte_off = mem_ref_offset (base_addr); + poly_offset_int bit_off = byte_off << LOG2_BITS_PER_UNIT; bit_off += bitpos; - if (!wi::neg_p (bit_off) && wi::fits_shwi_p (bit_off)) + if (known_ge (bit_off, 0) && bit_off.to_shwi (&bitpos)) { - bitpos = bit_off.to_shwi (); - if (bitregion_end) + if (maybe_ne (bitregion_end, 0U)) { bit_off = byte_off << LOG2_BITS_PER_UNIT; bit_off += bitregion_start; - if (wi::fits_uhwi_p (bit_off)) + if (bit_off.to_uhwi (&bitregion_start)) { - bitregion_start = bit_off.to_uhwi (); bit_off = byte_off << LOG2_BITS_PER_UNIT; bit_off += bitregion_end; - if (wi::fits_uhwi_p (bit_off)) - bitregion_end = bit_off.to_uhwi (); - else + if (!bit_off.to_uhwi (&bitregion_end)) bitregion_end = 0; } else @@ -3882,15 +3878,15 @@ mem_valid_for_store_merging (tree mem, u address now. */ else { - if (bitpos < 0) + if (maybe_lt (bitpos, 0)) return NULL_TREE; base_addr = build_fold_addr_expr (base_addr); } - if (!bitregion_end) + if (known_eq (bitregion_end, 0U)) { - bitregion_start = ROUND_DOWN (bitpos, BITS_PER_UNIT); - bitregion_end = ROUND_UP (bitpos + bitsize, BITS_PER_UNIT); + bitregion_start = round_down_to_byte_boundary (bitpos); + bitregion_end = round_up_to_byte_boundary (bitpos + bitsize); } if (offset != NULL_TREE) @@ -3922,9 +3918,8 @@ mem_valid_for_store_merging (tree mem, u static bool handled_load (gimple *stmt, store_operand_info *op, - unsigned HOST_WIDE_INT bitsize, unsigned HOST_WIDE_INT bitpos, - unsigned HOST_WIDE_INT bitregion_start, - unsigned HOST_WIDE_INT bitregion_end) + poly_uint64 bitsize, poly_uint64 bitpos, + poly_uint64 bitregion_start, poly_uint64 bitregion_end) { if (!is_gimple_assign (stmt)) return false; @@ -3956,10 +3951,12 @@ handled_load (gimple *stmt, store_operan &op->bitregion_start, &op->bitregion_end); if (op->base_addr != NULL_TREE - && op->bitsize == bitsize - && ((op->bitpos - bitpos) % BITS_PER_UNIT) == 0 - && op->bitpos - op->bitregion_start >= bitpos - bitregion_start - && op->bitregion_end - op->bitpos >= bitregion_end - bitpos) + && known_eq (op->bitsize, bitsize) + && multiple_p (op->bitpos - bitpos, BITS_PER_UNIT) + && known_ge (op->bitpos - op->bitregion_start, + bitpos - bitregion_start) + && known_ge (op->bitregion_end - op->bitpos, + bitregion_end - bitpos)) { op->stmt = stmt; op->val = mem; @@ -3978,18 +3975,18 @@ pass_store_merging::process_store (gimpl { tree lhs = gimple_assign_lhs (stmt); tree rhs = gimple_assign_rhs1 (stmt); - unsigned HOST_WIDE_INT bitsize, bitpos; - unsigned HOST_WIDE_INT bitregion_start; - unsigned HOST_WIDE_INT bitregion_end; + poly_uint64 bitsize, bitpos; + poly_uint64 bitregion_start, bitregion_end; tree base_addr = mem_valid_for_store_merging (lhs, &bitsize, &bitpos, &bitregion_start, &bitregion_end); - if (bitsize == 0) + if (known_eq (bitsize, 0U)) return; bool invalid = (base_addr == NULL_TREE - || ((bitsize > MAX_BITSIZE_MODE_ANY_INT) - && (TREE_CODE (rhs) != INTEGER_CST))); + || (maybe_gt (bitsize, + (unsigned int) MAX_BITSIZE_MODE_ANY_INT) + && (TREE_CODE (rhs) != INTEGER_CST))); enum tree_code rhs_code = ERROR_MARK; bool bit_not_p = false; struct symbolic_number n; @@ -4058,9 +4055,11 @@ pass_store_merging::process_store (gimpl invalid = true; break; } - if ((bitsize % BITS_PER_UNIT) == 0 - && (bitpos % BITS_PER_UNIT) == 0 - && bitsize <= 64 + unsigned HOST_WIDE_INT const_bitsize; + if (bitsize.is_constant (&const_bitsize) + && multiple_p (const_bitsize, BITS_PER_UNIT) + && multiple_p (bitpos, BITS_PER_UNIT) + && const_bitsize <= 64 && BYTES_BIG_ENDIAN == WORDS_BIG_ENDIAN) { ins_stmt = find_bswap_or_nop_1 (def_stmt, &n, 12); @@ -4068,7 +4067,8 @@ pass_store_merging::process_store (gimpl { uint64_t nn = n.n; for (unsigned HOST_WIDE_INT i = 0; - i < bitsize; i += BITS_PER_UNIT, nn >>= BITS_PER_MARKER) + i < const_bitsize; + i += BITS_PER_UNIT, nn >>= BITS_PER_MARKER) if ((nn & MARKER_MASK) == 0 || (nn & MARKER_MASK) == MARKER_BYTE_UNKNOWN) { @@ -4089,7 +4089,13 @@ pass_store_merging::process_store (gimpl } } - if (invalid) + unsigned HOST_WIDE_INT const_bitsize, const_bitpos; + unsigned HOST_WIDE_INT const_bitregion_start, const_bitregion_end; + if (invalid + || !bitsize.is_constant (&const_bitsize) + || !bitpos.is_constant (&const_bitpos) + || !bitregion_start.is_constant (&const_bitregion_start) + || !bitregion_end.is_constant (&const_bitregion_end)) { terminate_all_aliasing_chains (NULL, stmt); return; @@ -4106,9 +4112,10 @@ pass_store_merging::process_store (gimpl if (chain_info) { unsigned int ord = (*chain_info)->m_store_info.length (); - info = new store_immediate_info (bitsize, bitpos, bitregion_start, - bitregion_end, stmt, ord, rhs_code, - n, ins_stmt, + info = new store_immediate_info (const_bitsize, const_bitpos, + const_bitregion_start, + const_bitregion_end, + stmt, ord, rhs_code, n, ins_stmt, bit_not_p, ops[0], ops[1]); if (dump_file && (dump_flags & TDF_DETAILS)) { @@ -4135,9 +4142,10 @@ pass_store_merging::process_store (gimpl /* Start a new chain. */ struct imm_store_chain_info *new_chain = new imm_store_chain_info (m_stores_head, base_addr); - info = new store_immediate_info (bitsize, bitpos, bitregion_start, - bitregion_end, stmt, 0, rhs_code, - n, ins_stmt, + info = new store_immediate_info (const_bitsize, const_bitpos, + const_bitregion_start, + const_bitregion_end, + stmt, 0, rhs_code, n, ins_stmt, bit_not_p, ops[0], ops[1]); new_chain->m_store_info.safe_push (info); m_stores.put (base_addr, new_chain);