From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22394 invoked by alias); 26 Oct 2017 12:18:45 -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 22193 invoked by uid 89); 26 Oct 2017 12:18:45 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-14.8 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_ASCII_DIVIDERS,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=touches X-HELO: mail-wm0-f47.google.com Received: from mail-wm0-f47.google.com (HELO mail-wm0-f47.google.com) (74.125.82.47) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 26 Oct 2017 12:18:41 +0000 Received: by mail-wm0-f47.google.com with SMTP id m72so19254897wmc.0 for ; Thu, 26 Oct 2017 05:18:41 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to; bh=I9rUfEZ3rtQwk9TNxa+4IAYUaFTrjwVEL0XHTSqPiAc=; b=ooBC1YtxPAZl8rIvLZmvFD/r+T3GX4zC/he5GclX7/mOOX8DwIcybX+7ue2DQZ+DUR RxJo/zhwIC3ebDjTrnpOhNhx777wdcJC34KZAr0tpONw1NepZY1eAbjf0rdYjDlmFM6L vHI1NBkCETOrXRoA/LvMWDwTfiRA/5e1AvAmlKkzW1LS6hLsxI5BAXAI0tuVckyaWKub 8xXaRhWfwLgJS8LcQ93w1LIba6vBkT1Ash4wjDrgGSQcSwUGXP4yRuWEniLdfEYeA2xo zl4ERn4NgJ7GQ1Rji9BlnhO5cn/BlaokcJu99o+Qh4b86BAn/xSrGQ3fOlJh1FH954YK hh1Q== X-Gm-Message-State: AMCzsaUtEnJgWVUZbezqJQXUuk+ivrewAAEvG283rTd1PC1j9n7naCc4 mM/hKOYkFXVWARwHKslQN0nab1HFkPrwl+gtPuaCtA== X-Google-Smtp-Source: ABhQp+QD54ifh46YBXQBkCJTiBVVa4aleD3aaUCCA7tRthCx/kex1rOH11GvnU0uASzP9F97X4RHFYSoYGhbeDqK4Lk= X-Received: by 10.80.217.15 with SMTP id t15mr28359857edj.217.1509020318941; Thu, 26 Oct 2017 05:18:38 -0700 (PDT) MIME-Version: 1.0 Received: by 10.80.143.34 with HTTP; Thu, 26 Oct 2017 05:18:38 -0700 (PDT) In-Reply-To: <87a80iummr.fsf@linaro.org> References: <87wp3mxgir.fsf@linaro.org> <87a80iummr.fsf@linaro.org> From: Richard Biener Date: Thu, 26 Oct 2017 12:18:00 -0000 Message-ID: Subject: Re: [22/nn] Make dse.c use offset/width instead of start/end To: GCC Patches , Richard Sandiford Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2017-10/txt/msg01929.txt.bz2 On Mon, Oct 23, 2017 at 1:30 PM, Richard Sandiford wrote: > store_info and read_info_type in dse.c represented the ranges as > start/end, but a lot of the internal code used offset/width instead. > Using offset/width throughout fits better with the poly_int.h > range-checking functions. Ok. Richard. > > 2017-10-23 Richard Sandiford > Alan Hayward > David Sherwood > > gcc/ > * dse.c (store_info, read_info_type): Replace begin and end with > offset and width. > (print_range): New function. > (set_all_positions_unneeded, any_positions_needed_p) > (check_mem_read_rtx, scan_stores, scan_reads, dse_step5): Update > accordingly. > (record_store): Likewise. Optimize the case in which all positions > are unneeded. > (get_stored_val): Replace read_begin and read_end with read_offset > and read_width. > (replace_read): Update call accordingly. > > Index: gcc/dse.c > =================================================================== > --- gcc/dse.c 2017-10-23 11:47:11.273428262 +0100 > +++ gcc/dse.c 2017-10-23 11:47:48.294155952 +0100 > @@ -243,9 +243,12 @@ struct store_info > /* Canonized MEM address for use by canon_true_dependence. */ > rtx mem_addr; > > - /* The offset of the first and byte before the last byte associated > - with the operation. */ > - HOST_WIDE_INT begin, end; > + /* The offset of the first byte associated with the operation. */ > + HOST_WIDE_INT offset; > + > + /* The number of bytes covered by the operation. This is always exact > + and known (rather than -1). */ > + HOST_WIDE_INT width; > > union > { > @@ -261,7 +264,7 @@ struct store_info > bitmap bmap; > > /* Number of set bits (i.e. unneeded bytes) in BITMAP. If it is > - equal to END - BEGIN, the whole store is unused. */ > + equal to WIDTH, the whole store is unused. */ > int count; > } large; > } positions_needed; > @@ -304,10 +307,11 @@ struct read_info_type > /* The id of the mem group of the base address. */ > int group_id; > > - /* The offset of the first and byte after the last byte associated > - with the operation. If begin == end == 0, the read did not have > - a constant offset. */ > - int begin, end; > + /* The offset of the first byte associated with the operation. */ > + HOST_WIDE_INT offset; > + > + /* The number of bytes covered by the operation, or -1 if not known. */ > + HOST_WIDE_INT width; > > /* The mem being read. */ > rtx mem; > @@ -586,6 +590,18 @@ static deferred_change *deferred_change_ > > /* The number of bits used in the global bitmaps. */ > static unsigned int current_position; > + > +/* Print offset range [OFFSET, OFFSET + WIDTH) to FILE. */ > + > +static void > +print_range (FILE *file, poly_int64 offset, poly_int64 width) > +{ > + fprintf (file, "["); > + print_dec (offset, file, SIGNED); > + fprintf (file, ".."); > + print_dec (offset + width, file, SIGNED); > + fprintf (file, ")"); > +} > > /*---------------------------------------------------------------------------- > Zeroth step. > @@ -1212,10 +1228,9 @@ set_all_positions_unneeded (store_info * > { > if (__builtin_expect (s_info->is_large, false)) > { > - int pos, end = s_info->end - s_info->begin; > - for (pos = 0; pos < end; pos++) > - bitmap_set_bit (s_info->positions_needed.large.bmap, pos); > - s_info->positions_needed.large.count = end; > + bitmap_set_range (s_info->positions_needed.large.bmap, > + 0, s_info->width); > + s_info->positions_needed.large.count = s_info->width; > } > else > s_info->positions_needed.small_bitmask = HOST_WIDE_INT_0U; > @@ -1227,8 +1242,7 @@ set_all_positions_unneeded (store_info * > any_positions_needed_p (store_info *s_info) > { > if (__builtin_expect (s_info->is_large, false)) > - return (s_info->positions_needed.large.count > - < s_info->end - s_info->begin); > + return s_info->positions_needed.large.count < s_info->width; > else > return (s_info->positions_needed.small_bitmask != HOST_WIDE_INT_0U); > } > @@ -1355,8 +1369,12 @@ record_store (rtx body, bb_info_t bb_inf > set_usage_bits (group, offset, width, expr); > > if (dump_file && (dump_flags & TDF_DETAILS)) > - fprintf (dump_file, " processing const base store gid=%d[%d..%d)\n", > - group_id, (int)offset, (int)(offset+width)); > + { > + fprintf (dump_file, " processing const base store gid=%d", > + group_id); > + print_range (dump_file, offset, width); > + fprintf (dump_file, "\n"); > + } > } > else > { > @@ -1368,8 +1386,11 @@ record_store (rtx body, bb_info_t bb_inf > group_id = -1; > > if (dump_file && (dump_flags & TDF_DETAILS)) > - fprintf (dump_file, " processing cselib store [%d..%d)\n", > - (int)offset, (int)(offset+width)); > + { > + fprintf (dump_file, " processing cselib store "); > + print_range (dump_file, offset, width); > + fprintf (dump_file, "\n"); > + } > } > > const_rhs = rhs = NULL_RTX; > @@ -1435,18 +1456,21 @@ record_store (rtx body, bb_info_t bb_inf > { > HOST_WIDE_INT i; > if (dump_file && (dump_flags & TDF_DETAILS)) > - fprintf (dump_file, " trying store in insn=%d gid=%d[%d..%d)\n", > - INSN_UID (ptr->insn), s_info->group_id, > - (int)s_info->begin, (int)s_info->end); > + { > + fprintf (dump_file, " trying store in insn=%d gid=%d", > + INSN_UID (ptr->insn), s_info->group_id); > + print_range (dump_file, s_info->offset, s_info->width); > + fprintf (dump_file, "\n"); > + } > > /* Even if PTR won't be eliminated as unneeded, if both > PTR and this insn store the same constant value, we might > eliminate this insn instead. */ > if (s_info->const_rhs > && const_rhs > - && offset >= s_info->begin > - && offset + width <= s_info->end > - && all_positions_needed_p (s_info, offset - s_info->begin, > + && known_subrange_p (offset, width, > + s_info->offset, s_info->width) > + && all_positions_needed_p (s_info, offset - s_info->offset, > width)) > { > if (GET_MODE (mem) == BLKmode) > @@ -1462,8 +1486,7 @@ record_store (rtx body, bb_info_t bb_inf > { > rtx val; > start_sequence (); > - val = get_stored_val (s_info, GET_MODE (mem), > - offset, offset + width, > + val = get_stored_val (s_info, GET_MODE (mem), offset, width, > BLOCK_FOR_INSN (insn_info->insn), > true); > if (get_insns () != NULL) > @@ -1474,10 +1497,18 @@ record_store (rtx body, bb_info_t bb_inf > } > } > > - for (i = MAX (offset, s_info->begin); > - i < offset + width && i < s_info->end; > - i++) > - set_position_unneeded (s_info, i - s_info->begin); > + if (known_subrange_p (s_info->offset, s_info->width, offset, width)) > + /* The new store touches every byte that S_INFO does. */ > + set_all_positions_unneeded (s_info); > + else > + { > + HOST_WIDE_INT begin_unneeded = offset - s_info->offset; > + HOST_WIDE_INT end_unneeded = begin_unneeded + width; > + begin_unneeded = MAX (begin_unneeded, 0); > + end_unneeded = MIN (end_unneeded, s_info->width); > + for (i = begin_unneeded; i < end_unneeded; ++i) > + set_position_unneeded (s_info, i); > + } > } > else if (s_info->rhs) > /* Need to see if it is possible for this store to overwrite > @@ -1535,8 +1566,8 @@ record_store (rtx body, bb_info_t bb_inf > store_info->positions_needed.small_bitmask = lowpart_bitmask (width); > } > store_info->group_id = group_id; > - store_info->begin = offset; > - store_info->end = offset + width; > + store_info->offset = offset; > + store_info->width = width; > store_info->is_set = GET_CODE (body) == SET; > store_info->rhs = rhs; > store_info->const_rhs = const_rhs; > @@ -1700,39 +1731,38 @@ look_for_hardregs (rtx x, const_rtx pat > } > > /* Helper function for replace_read and record_store. > - Attempt to return a value stored in STORE_INFO, from READ_BEGIN > - to one before READ_END bytes read in READ_MODE. Return NULL > + Attempt to return a value of mode READ_MODE stored in STORE_INFO, > + consisting of READ_WIDTH bytes starting from READ_OFFSET. Return NULL > if not successful. If REQUIRE_CST is true, return always constant. */ > > static rtx > get_stored_val (store_info *store_info, machine_mode read_mode, > - HOST_WIDE_INT read_begin, HOST_WIDE_INT read_end, > + HOST_WIDE_INT read_offset, HOST_WIDE_INT read_width, > basic_block bb, bool require_cst) > { > machine_mode store_mode = GET_MODE (store_info->mem); > - int shift; > - int access_size; /* In bytes. */ > + HOST_WIDE_INT gap; > rtx read_reg; > > /* To get here the read is within the boundaries of the write so > shift will never be negative. Start out with the shift being in > bytes. */ > if (store_mode == BLKmode) > - shift = 0; > + gap = 0; > else if (BYTES_BIG_ENDIAN) > - shift = store_info->end - read_end; > + gap = ((store_info->offset + store_info->width) > + - (read_offset + read_width)); > else > - shift = read_begin - store_info->begin; > - > - access_size = shift + GET_MODE_SIZE (read_mode); > - > - /* From now on it is bits. */ > - shift *= BITS_PER_UNIT; > + gap = read_offset - store_info->offset; > > - if (shift) > - read_reg = find_shift_sequence (access_size, store_info, read_mode, shift, > - optimize_bb_for_speed_p (bb), > - require_cst); > + if (gap != 0) > + { > + HOST_WIDE_INT shift = gap * BITS_PER_UNIT; > + HOST_WIDE_INT access_size = GET_MODE_SIZE (read_mode) + gap; > + read_reg = find_shift_sequence (access_size, store_info, read_mode, > + shift, optimize_bb_for_speed_p (bb), > + require_cst); > + } > else if (store_mode == BLKmode) > { > /* The store is a memset (addr, const_val, const_size). */ > @@ -1835,7 +1865,7 @@ replace_read (store_info *store_info, in > start_sequence (); > bb = BLOCK_FOR_INSN (read_insn->insn); > read_reg = get_stored_val (store_info, > - read_mode, read_info->begin, read_info->end, > + read_mode, read_info->offset, read_info->width, > bb, false); > if (read_reg == NULL_RTX) > { > @@ -1986,8 +2016,8 @@ check_mem_read_rtx (rtx *loc, bb_info_t > read_info = read_info_type_pool.allocate (); > read_info->group_id = group_id; > read_info->mem = mem; > - read_info->begin = offset; > - read_info->end = offset + width; > + read_info->offset = offset; > + read_info->width = width; > read_info->next = insn_info->read_rec; > insn_info->read_rec = read_info; > if (group_id < 0) > @@ -2013,8 +2043,11 @@ check_mem_read_rtx (rtx *loc, bb_info_t > fprintf (dump_file, " processing const load gid=%d[BLK]\n", > group_id); > else > - fprintf (dump_file, " processing const load gid=%d[%d..%d)\n", > - group_id, (int)offset, (int)(offset+width)); > + { > + fprintf (dump_file, " processing const load gid=%d", group_id); > + print_range (dump_file, offset, width); > + fprintf (dump_file, "\n"); > + } > } > > while (i_ptr) > @@ -2052,19 +2085,19 @@ check_mem_read_rtx (rtx *loc, bb_info_t > else > { > if (store_info->rhs > - && offset >= store_info->begin > - && offset + width <= store_info->end > + && known_subrange_p (offset, width, store_info->offset, > + store_info->width) > && all_positions_needed_p (store_info, > - offset - store_info->begin, > + offset - store_info->offset, > width) > && replace_read (store_info, i_ptr, read_info, > insn_info, loc, bb_info->regs_live)) > return; > > /* The bases are the same, just see if the offsets > - overlap. */ > - if ((offset < store_info->end) > - && (offset + width > store_info->begin)) > + could overlap. */ > + if (ranges_may_overlap_p (offset, width, store_info->offset, > + store_info->width)) > remove = true; > } > } > @@ -2119,11 +2152,10 @@ check_mem_read_rtx (rtx *loc, bb_info_t > if (store_info->rhs > && store_info->group_id == -1 > && store_info->cse_base == base > - && width != -1 > - && offset >= store_info->begin > - && offset + width <= store_info->end > + && known_subrange_p (offset, width, store_info->offset, > + store_info->width) > && all_positions_needed_p (store_info, > - offset - store_info->begin, width) > + offset - store_info->offset, width) > && replace_read (store_info, i_ptr, read_info, insn_info, loc, > bb_info->regs_live)) > return; > @@ -2775,16 +2807,19 @@ scan_stores (store_info *store_info, bit > group_info *group_info > = rtx_group_vec[store_info->group_id]; > if (group_info->process_globally) > - for (i = store_info->begin; i < store_info->end; i++) > - { > - int index = get_bitmap_index (group_info, i); > - if (index != 0) > - { > - bitmap_set_bit (gen, index); > - if (kill) > - bitmap_clear_bit (kill, index); > - } > - } > + { > + HOST_WIDE_INT end = store_info->offset + store_info->width; > + for (i = store_info->offset; i < end; i++) > + { > + int index = get_bitmap_index (group_info, i); > + if (index != 0) > + { > + bitmap_set_bit (gen, index); > + if (kill) > + bitmap_clear_bit (kill, index); > + } > + } > + } > store_info = store_info->next; > } > } > @@ -2834,9 +2869,9 @@ scan_reads (insn_info_t insn_info, bitma > { > if (i == read_info->group_id) > { > - if (read_info->begin > read_info->end) > + if (!known_size_p (read_info->width)) > { > - /* Begin > end for block mode reads. */ > + /* Handle block mode reads. */ > if (kill) > bitmap_ior_into (kill, group->group_kill); > bitmap_and_compl_into (gen, group->group_kill); > @@ -2846,7 +2881,8 @@ scan_reads (insn_info_t insn_info, bitma > /* The groups are the same, just process the > offsets. */ > HOST_WIDE_INT j; > - for (j = read_info->begin; j < read_info->end; j++) > + HOST_WIDE_INT end = read_info->offset + read_info->width; > + for (j = read_info->offset; j < end; j++) > { > int index = get_bitmap_index (group, j); > if (index != 0) > @@ -3265,7 +3301,8 @@ dse_step5 (void) > HOST_WIDE_INT i; > group_info *group_info = rtx_group_vec[store_info->group_id]; > > - for (i = store_info->begin; i < store_info->end; i++) > + HOST_WIDE_INT end = store_info->offset + store_info->width; > + for (i = store_info->offset; i < end; i++) > { > int index = get_bitmap_index (group_info, i); >