From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 112982 invoked by alias); 11 Oct 2016 09:26: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 112220 invoked by uid 89); 11 Oct 2016 09:26:45 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_PASS autolearn=ham version=3.3.2 spammy=Ready, gimple_location, sk:build_p 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; Tue, 11 Oct 2016 09:26:35 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id AFA7FAC86; Tue, 11 Oct 2016 09:26:33 +0000 (UTC) Subject: Re: [PATCH 1/3] Fold __builtin_str{n}{case}cmp functions (version 2) To: Richard Biener References: <678ff58e-4aa3-6145-f56b-780bf618338c@suse.cz> Cc: GCC Patches From: =?UTF-8?Q?Martin_Li=c5=a1ka?= Message-ID: <8b542e2d-8e49-8073-8f0c-25500b965341@suse.cz> Date: Tue, 11 Oct 2016 09:26:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2016-10/txt/msg00689.txt.bz2 On 10/07/2016 12:50 PM, Richard Biener wrote: > On Fri, Oct 7, 2016 at 10:39 AM, Martin Liška wrote: >> I'm resending the patch, where I implemented all builtins mentions in subject >> in gimp-fold.c. >> >> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >> >> Ready to be installed? > > + case BUILT_IN_STRNCASECMP: > + { > + r = strncmp (p1, p2, length); > + if (r == 0) > + known_result = true; > > length might be -1 here -- I think you need to guard against that (but you can > handle BUILT_IN_STRCASECMP which you miss?). Likewise for the strncmp case. Fixed, I've added comment to STRCASECMP case. > > Also do we know whether the c_getstr () result is '\0'-terminated? AFAICS these > constant foldings were not implemented in builtins.c, I see a STRCMP one in > fold-const-call.c though. I believe the STRING_CST string is not guaranteed to > be '\0' terminated (STRING_CST comes with explicit length). You are absolutely right that we do not have always '\0'-terminated string constants. Thus I'll send a patch that would return a string from c_getstr just in case string[string_length] == 0 (separate email with patch will be sent). > > + tree temp = fold_build2_loc (loc, MEM_REF, cst_uchar_node, str1, > + off0); > + temp = gimple_build (&stmts, loc, NOP_EXPR, cst_uchar_node, temp); > > please don't use gimple_build here, there is nothing to simplify for it. Using > a NOP_EXPR is also confusing (to match the API...). Just do > gimple_build_assign (make_ssa_name (...), ..) like other folders do. > > + replace_call_with_value (gsi, fold_convert_loc (loc, type, temp)); > > and you'd want to replace the call with the MEM_REF stmt using > gsi_replace_with_seq_vops as you fail to set virtual operands properly > above (thus you'll get ICEs when this only matches during GIMPLE opts). > > + location_t loc = gimple_location (stmt); > + tree cst_uchar_node = build_type_variant (unsigned_char_type_node, 1, 0); > + tree cst_uchar_ptr_node > + = build_pointer_type_for_mode (cst_uchar_node, ptr_mode, true); > + tree off0 = build_int_cst (cst_uchar_ptr_node, 0); > > it would be nice to not do this tree building if nothign is folded. > > + case BUILT_IN_STRCMP: > + return gimple_fold_builtin_string_compare (gsi); > + case BUILT_IN_STRCASECMP: > + return gimple_fold_builtin_string_compare (gsi); > + case BUILT_IN_STRNCMP: > + return gimple_fold_builtin_string_compare (gsi); > + case BUILT_IN_STRNCASECMP: > + return gimple_fold_builtin_string_compare (gsi); > > please do > > + case BUILT_IN_STRCMP: > + case BUILT_IN_STRCASECMP: > ... > + return gimple_fold_builtin_string_compare (gsi); > > Thanks, > Richard. Sure, all notes will be fixed in an email which reply to this one. Martin > >> Martin