From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 41976 invoked by alias); 21 Mar 2018 10:11:06 -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 41949 invoked by uid 89); 21 Mar 2018 10:11:02 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.9 required=5.0 tests=BAYES_00,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= X-HELO: mx2.suse.de Received: from mx2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 21 Mar 2018 10:11:01 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id D5490AF89; Wed, 21 Mar 2018 10:10:58 +0000 (UTC) Date: Wed, 21 Mar 2018 10:26:00 -0000 From: Richard Biener To: Jakub Jelinek cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] Fix up handling of bool BIT_NOT_EXPRs in store-merging (PR tree-optimization/84982, take 2) In-Reply-To: <20180321100710.GH8577@tucnak> Message-ID: References: <20180320204224.GD8577@tucnak> <20180321100710.GH8577@tucnak> User-Agent: Alpine 2.20 (LSU 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-SW-Source: 2018-03/txt/msg01075.txt.bz2 On Wed, 21 Mar 2018, Jakub Jelinek wrote: > On Wed, Mar 21, 2018 at 09:20:40AM +0100, Richard Biener wrote: > > > + tree lhs = gimple_assign_lhs (info->stmt); > > > + if (TREE_CODE (TREE_TYPE (lhs)) == BOOLEAN_TYPE && info->bitsize > 1) > > > > So I'm slightly uncomfortable with keying this just on BOOLEAN_TYPE. > > Do you think anything would go wrong with simply using > > > > if (INTEGRAL_TYPE_P (TREE_TYPE (lhs)) > > && TYPE_PRECISION (TREE_TYPE (lhs)) < info->bitsize) > > > > ? > > > > It would then be any_padding rather than any_bool. > > So like this? The assembly for the testcase is still identical to previous > patch on both x86_64-linux and powerpc-linux. Yes. > I've tried to reproduce the case with non-bool integral types with precision > smaller than size, but even with C++ -fstrict-enums haven't succeeded, those > enums have different TYPE_*_VALUE, but TYPE_PRECISION is still equal to the > TYPE_SIZE, in the end I've just changed in gdb TYPE_PRECISION of the > enumerated type and checked that there is (without store merging) xor with > the mask of only precision bits emitted. Perhaps in Ada one can construct > something? I don't speak Ada though... Yeah, I'm just fearing there are no rules prohibiting such types ;) Like for C++ and -fstrict-enums we have constrained TYPE_MIN/MAX_VALUE but not TYPE_PRECISION (for whatever reason ...). Patch is ok. Richard. > 2018-03-21 Jakub Jelinek > > PR tree-optimization/84982 > * gimple-ssa-store-merging.c (invert_op): Handle boolean inversion > by flipping the least significant bit rather than all bits from > bitpos to bitpos + bitsize - 1. > > * c-c++-common/pr84982.c: New test. > > --- gcc/gimple-ssa-store-merging.c.jj 2018-03-20 22:05:54.368430762 +0100 > +++ gcc/gimple-ssa-store-merging.c 2018-03-21 10:45:39.919458647 +0100 > @@ -3248,16 +3248,23 @@ invert_op (split_store *split_store, int > unsigned int i; > store_immediate_info *info; > unsigned int cnt = 0; > + bool any_paddings = false; > FOR_EACH_VEC_ELT (split_store->orig_stores, i, info) > { > bool bit_not_p = idx < 2 ? info->ops[idx].bit_not_p : info->bit_not_p; > if (bit_not_p) > - ++cnt; > + { > + ++cnt; > + tree lhs = gimple_assign_lhs (info->stmt); > + if (INTEGRAL_TYPE_P (TREE_TYPE (lhs)) > + && TYPE_PRECISION (TREE_TYPE (lhs)) < info->bitsize) > + any_paddings = true; > + } > } > mask = NULL_TREE; > if (cnt == 0) > return NOP_EXPR; > - if (cnt == split_store->orig_stores.length ()) > + if (cnt == split_store->orig_stores.length () && !any_paddings) > return BIT_NOT_EXPR; > > unsigned HOST_WIDE_INT try_bitpos = split_store->bytepos * BITS_PER_UNIT; > @@ -3274,14 +3281,42 @@ invert_op (split_store *split_store, int > clear regions with !bit_not_p, so that gaps in between stores aren't > set in the mask. */ > unsigned HOST_WIDE_INT bitsize = info->bitsize; > + unsigned HOST_WIDE_INT prec = bitsize; > unsigned int pos_in_buffer = 0; > + if (any_paddings) > + { > + tree lhs = gimple_assign_lhs (info->stmt); > + if (INTEGRAL_TYPE_P (TREE_TYPE (lhs)) > + && TYPE_PRECISION (TREE_TYPE (lhs)) < bitsize) > + prec = TYPE_PRECISION (TREE_TYPE (lhs)); > + } > if (info->bitpos < try_bitpos) > { > gcc_assert (info->bitpos + bitsize > try_bitpos); > - bitsize -= (try_bitpos - info->bitpos); > + if (!BYTES_BIG_ENDIAN) > + { > + if (prec <= try_bitpos - info->bitpos) > + continue; > + prec -= try_bitpos - info->bitpos; > + } > + bitsize -= try_bitpos - info->bitpos; > + if (BYTES_BIG_ENDIAN && prec > bitsize) > + prec = bitsize; > } > else > pos_in_buffer = info->bitpos - try_bitpos; > + if (prec < bitsize) > + { > + /* If this is a bool inversion, invert just the least significant > + prec bits rather than all bits of it. */ > + if (BYTES_BIG_ENDIAN) > + { > + pos_in_buffer += bitsize - prec; > + if (pos_in_buffer >= split_store->size) > + continue; > + } > + bitsize = prec; > + } > if (pos_in_buffer + bitsize > split_store->size) > bitsize = split_store->size - pos_in_buffer; > unsigned char *p = buf + (pos_in_buffer / BITS_PER_UNIT); > --- gcc/testsuite/c-c++-common/pr84982.c.jj 2018-03-20 14:49:00.259744750 +0100 > +++ gcc/testsuite/c-c++-common/pr84982.c 2018-03-20 12:27:34.111363552 +0100 > @@ -0,0 +1,38 @@ > +/* PR tree-optimization/84982 */ > +/* { dg-do run } */ > +/* { dg-options "-O2" } */ > + > +#ifndef __cplusplus > +#define bool _Bool > +#define true 1 > +#define false 0 > +#endif > + > +struct S { bool a, b, c, d; }; > + > +__attribute__((noipa)) void > +bar (bool *x) > +{ > + if (x[0] || !x[1] || !x[2] || x[3]) > + __builtin_abort (); > +} > + > +__attribute__((noipa)) void > +foo (struct S *x) > +{ > + bool a[4]; > + a[0] = !x->a; > + a[1] = !x->b; > + a[2] = x->c; > + a[3] = !x->d; > + bar (a); > +} > + > +int > +main () > +{ > + struct S s; > + s.a = true; s.b = false; s.c = true; s.d = true; > + foo (&s); > + return 0; > +} > > > Jakub > > -- Richard Biener SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)