From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 85028 invoked by alias); 11 Oct 2016 10:27: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 85013 invoked by uid 89); 11 Oct 2016 10:27:44 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=no version=3.3.2 spammy= X-HELO: mail-lf0-f68.google.com Received: from mail-lf0-f68.google.com (HELO mail-lf0-f68.google.com) (209.85.215.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 11 Oct 2016 10:27:34 +0000 Received: by mail-lf0-f68.google.com with SMTP id l131so221673lfl.0 for ; Tue, 11 Oct 2016 03:27:33 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=JuTPgwwqmLQRKgVZA7TxW7QB5WeEJ3nAcbDTVZnbv8o=; b=fM7LxdI2Wd1IfNfmkVlpnPD5kQVILQ6bfHaLsxI5EkJOtDv9zYfsrUn6eepSLs/t4Y jbogChZyeDsByagVd6h1Mlb7o6xFr67F+EyQ5nj1yycLIxJkyst/LXSM2jYiNZ0spxkH s+gKmkMCy75tB6jVCNEjI7JirTp99NHhhBxTa1hcnBoOFNrS3g4OyaVJsEsYEgVv6c// W8inbD+Zb7ovNk2Mm/F7ej+gxAzk9F+4jyhxrE4rQqmgWMohlPKBPxrobKIlM7MX3zAO dnBKDmDLB1NKLqXb8dYIObCwRLHr4l8KNP4hNILPhdgHq6SLX7F93vWxHFyxtaRP8LLb DRJA== X-Gm-Message-State: AA6/9RlfPfdMY0xUyVMlopKFXpJxHFzJPXAyEAWnmap/5yilUJId2n8uoF/QUi8GyymBZFEEsPHv2nfdhcpsPw== X-Received: by 10.194.235.103 with SMTP id ul7mr4041028wjc.201.1476181651935; Tue, 11 Oct 2016 03:27:31 -0700 (PDT) MIME-Version: 1.0 Received: by 10.28.155.146 with HTTP; Tue, 11 Oct 2016 03:27:31 -0700 (PDT) In-Reply-To: <8b542e2d-8e49-8073-8f0c-25500b965341@suse.cz> References: <678ff58e-4aa3-6145-f56b-780bf618338c@suse.cz> <8b542e2d-8e49-8073-8f0c-25500b965341@suse.cz> From: Richard Biener Date: Tue, 11 Oct 2016 10:27:00 -0000 Message-ID: Subject: Re: [PATCH 1/3] Fold __builtin_str{n}{case}cmp functions (version 2) To: =?UTF-8?Q?Martin_Li=C5=A1ka?= Cc: GCC Patches Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2016-10/txt/msg00701.txt.bz2 On Tue, Oct 11, 2016 at 11:26 AM, Martin Li=C5=A1ka wrote: > On 10/07/2016 12:50 PM, Richard Biener wrote: >> On Fri, Oct 7, 2016 at 10:39 AM, Martin Li=C5=A1ka wrot= e: >>> I'm resending the patch, where I implemented all builtins mentions in s= ubject >>> in gimp-fold.c. >>> >>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tes= ts. >>> >>> Ready to be installed? >> >> + case BUILT_IN_STRNCASECMP: >> + { >> + r =3D strncmp (p1, p2, length); >> + if (r =3D=3D 0) >> + known_result =3D true; >> >> length might be -1 here -- I think you need to guard against that (but y= ou can >> handle BUILT_IN_STRCASECMP which you miss?). Likewise for the strncmp c= ase. > > Fixed, I've added comment to STRCASECMP case. > >> >> Also do we know whether the c_getstr () result is '\0'-terminated? AFAI= CS 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 guaran= teed to >> be '\0' terminated (STRING_CST comes with explicit length). > > You are absolutely right that we do not have always '\0'-terminated strin= g constants. > Thus I'll send a patch that would return a string from c_getstr just in c= ase > string[string_length] =3D=3D 0 (separate email with patch will be sent). Maybe add a second output to c_getstr to pass down the string length in case it is known? In this case you could use strN* () variants for constant folding. "not found" would need to be folded conditional on null termination to avoid folding undefined behavior. Richard. >> >> + tree temp =3D fold_build2_loc (loc, MEM_REF, cst_uchar_node, str1, >> + off0); >> + temp =3D gimple_build (&stmts, loc, NOP_EXPR, cst_uchar_node, tem= p); >> >> 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 =3D gimple_location (stmt); >> + tree cst_uchar_node =3D build_type_variant (unsigned_char_type_node, = 1, 0); >> + tree cst_uchar_ptr_node >> + =3D build_pointer_type_for_mode (cst_uchar_node, ptr_mode, true); >> + tree off0 =3D 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 >