From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 102117 invoked by alias); 15 Dec 2017 20:49:46 -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 102100 invoked by uid 89); 15 Dec 2017 20:49:45 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=BAYES_00,KAM_NUMSUBJECT,T_RP_MATCHES_RCVD,UNPARSEABLE_RELAY autolearn=no version=3.3.2 spammy= X-HELO: userp2130.oracle.com Received: from userp2130.oracle.com (HELO userp2130.oracle.com) (156.151.31.86) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 15 Dec 2017 20:49:43 +0000 Received: from pps.filterd (userp2130.oracle.com [127.0.0.1]) by userp2130.oracle.com (8.16.0.21/8.16.0.21) with SMTP id vBFKklKC191968; Fri, 15 Dec 2017 20:49:41 GMT Received: from userv0022.oracle.com (userv0022.oracle.com [156.151.31.74]) by userp2130.oracle.com with ESMTP id 2evnpv810w-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 15 Dec 2017 20:49:41 +0000 Received: from aserv0122.oracle.com (aserv0122.oracle.com [141.146.126.236]) by userv0022.oracle.com (8.14.4/8.14.4) with ESMTP id vBFKltMr024676 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 15 Dec 2017 20:47:56 GMT Received: from abhmp0007.oracle.com (abhmp0007.oracle.com [141.146.116.13]) by aserv0122.oracle.com (8.14.4/8.14.4) with ESMTP id vBFKlsQr026571; Fri, 15 Dec 2017 20:47:55 GMT Received: from dhcp-10-154-126-215.vpn.oracle.com (/10.154.126.215) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Fri, 15 Dec 2017 12:47:54 -0800 Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\)) Subject: Re: [PATCH][Middle-end]2nd patch of PR78809 and PR83026 From: Qing Zhao In-Reply-To: <20171215174749.GN2353@tucnak> Date: Fri, 15 Dec 2017 20:49:00 -0000 Cc: gcc-patches , jeff Law , richard Biener Content-Transfer-Encoding: quoted-printable Message-Id: References: <20171214204542.GB2353@tucnak> <20171215164245.GL2353@tucnak> <20171215174749.GN2353@tucnak> To: Jakub Jelinek X-Proofpoint-Virus-Version: vendor=nai engine=5900 definitions=8746 signatures=668648 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=3 malwarescore=0 phishscore=0 bulkscore=0 spamscore=0 mlxscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1711220000 definitions=main-1712150290 X-IsSubscribed: yes X-SW-Source: 2017-12/txt/msg01094.txt.bz2 > On Dec 15, 2017, at 11:47 AM, Jakub Jelinek wrote: >=20 > On Fri, Dec 15, 2017 at 11:17:37AM -0600, Qing Zhao wrote: >> HOST_WIDE_INT const_string_leni =3D -1; >>=20 >> if (idx1) >> { >> const_string_leni =3D compute_string_length (idx1); >> var_string =3D arg2; >> } >> else if (idx2) >> { >> const_string_leni =3D compute_string_length (idx2); >> var_string =3D arg1; >> } >>=20 >> so, the -Wmaybe-uninitialized should NOT issue warning, right? >=20 > Well, you had the var_string var uninitialized, so that is what I was > talking about. oh, yeah, forgot to initialize var_string. >=20 >> but anyway, I can change the above as following: >>=20 >> HOST_WIDE_INT const_string_leni =3D -1; >=20 > And here you don't need to initialize it. >=20 >> if (idx1) >> { >> const_string_leni =3D compute_string_length (idx1); >> var_string =3D arg2; >> } >> else >> { >> gcc_assert (idx2); >> const_string_leni =3D compute_string_length (idx2); >> var_string =3D arg1; >> } >>=20 >> is this better? >=20 > Yes, though the gcc_assert could be just gcc_checking_assert (idx2); what=E2=80=99s the major difference between these two assertion calls? >=20 >>> so that would be mode 2, though that >>> mode isn't actually used in real-world code and thus might be not fully >>> tested. >>=20 >> so, using this routine with mode 2 should be the right approach to go?=20 >> and we need fully testing on this too? >=20 > It has been a while since I wrote it, so it would need careful analysis. >=20 >>>> B. use =E2=80=9Cget_range_strlen=E2=80=9D in gimple-fold.h to decide = the size of the object. however,=20 >>>> it cannot provide valid info for simple array, either.=20 >>>=20 >>> get_range_strlen returns you a range, the minval is not what you're loo= king >>> for, that is the minimum string length, so might be too short for your >>> purposes. And maxval is an upper bound, but you are looking for lower >>> bound, you need guarantees this amount of memory can be accessed, even = if >>> there is 0 in the first byte. >>=20 >> my understanding is that: get_range_strlen returns the minimum and maxim= um length of the string pointed by the=20 >> pointer, and the maximum length of the string is determined by the size = of the allocated memory pointed by the >> pointer, so, it should serve my purpose, did I misunderstand it? >=20 > What I'm worried about is: > struct S { int a; char b[64]; }; > struct T { struct S c; char d; }; > int > foo (struct T *x) > { > return strcmp (x->c.b, "012345678901234567890123456789012345678901234567= 89") =3D=3D 0; > } > int > bar (void) > { > struct S *p =3D malloc (offsetof (struct S, b) + 8); > p->a =3D 123; > strcpy (p->b, "0123456"); > return foo ((struct T *) p); > } > etc. where if you transform that into memcmp (x->c.b, "01234567890123456= 789012345678901234567890123456789", 51) =3D=3D 0 > it will segfault, whereas strcmp would not. thanks for the example. is this issue only for the flexible array member?=20 if so, the current get_range_strlen will distinguish whether this is for fl= exible array member or not, then we can disable such transformation for flexible array member. >=20 >>> But if we find out during >>> expansion we don't want to expand it inline, we should fall back to cal= ling >>> strcmp or strncmp. >>=20 >> under what situation we will NOT expand the memcpy_eq call inline? >=20 > target =3D expand_builtin_memcmp (exp, target, fcode =3D=3D BUILT_IN= _MEMCMP_EQ); > if (target) > return target; > if (fcode =3D=3D BUILT_IN_MEMCMP_EQ) > { > tree newdecl =3D builtin_decl_explicit (BUILT_IN_MEMCMP); > TREE_OPERAND (exp, 1) =3D build_fold_addr_expr (newdecl); > } > is what builtins.c has, so it certainly counts with the possibility. > Now, both expand_builtin_memcmp, and emit_block_cmp_hints has several cas= es > when it fails. E.g. can_do_by_pieces decides it is too expensive to do it > inline, and emit_block_cmp_via_cmpmem fails because the target doesn't ha= ve > cmpmemsi expander. Various other cases. >=20 > Also, note that some target might have cmpstr*si expanders implemented, b= ut > not cmpmemsi, in which case trying to optimize strcmp as memcmp_eq might = be a > severe pessimization. Okay, I see. this is reasonable.=20 if the following better: (some details still need more work, just basic id= ea) in handle_builtin_string_cmp of tree-ssa-strlen.c: + /* Replace strcmp or strncmp with the BUILT_IN_STRCMP_EQ. */ + if (var_sizei > final_length)=20 + { + tree fn =3D builtin_decl_implicit (BUILT_IN_STRCMP_EQ/BUILT_IN_STRNC= MP_EQ; + if (!fn) + return true; + tree const_string_len =3D build_int_cst (size_type_node, final_lengt= h);=20 + update_gimple_call (gsi, fn, 3, arg1, arg2, const_string_len); + } then in builtins.c, add the following: case BUILT_IN_STRCMP_EQ: case BUILT_IN_STRNCMP_EQ: target =3D expand_builtin_memcmp (exp, target, fcode =3D=3D BUILT_IN_= MEMCMP_EQ); if (target) return target; else=20 { tree newdecl =3D builtin_decl_explicit (BUILT_IN_STRCMP/BUILT_IN_S= TRNCMP); TREE_OPERAND (exp, 1) =3D build_fold_addr_expr (newdecl); } =20=20=20=20=20=20 break; thanks. Qing >=20 > Jakub