From patchwork Mon Jun 24 23:50:12 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Sebor X-Patchwork-Id: 1121590 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-503639-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=gmail.com Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 45XmJ50P0nz9s9h for ; Tue, 25 Jun 2019 09:50:27 +1000 (AEST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to :from:subject:message-id:date:mime-version:content-type; q=dns; s=default; b=xUqRav/RY9X2DvVcm+eWe6xW4lMU5cBOKuuSrOB2GTOZcEBxso lQkMUHyky31YYClntndoMNOm+cYE4tuIfn1qUxH4s9kdENNfpotvcd1/v/rVM31r eJhorhVHSMerORipVYfmpqI/vK5dcwAv/Hk2tKAR32CKrmUu4nJ3fPd0c= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to :from:subject:message-id:date:mime-version:content-type; s= default; bh=GNY5mXHB8eCC2eMdHF8uUg6UruY=; b=iW+6K8r6F3qGgrxLuZcC U2jKNJnEXhSRgEBkTeGXH/Epq0mFbHo4DYwmPmrim7DBjFTJibBgvqpz2KCwIPn0 OtX2navOE9AmlKUYJ4LzNAoi0fPdpWYju5bfsusL0Eh9XfDw0NluuBMGQMlKy92D f2tSWEb3ol6MYsdsFbuqn1I= Received: (qmail 18647 invoked by alias); 24 Jun 2019 23:50:20 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 18114 invoked by uid 89); 24 Jun 2019 23:50:19 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-10.9 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_2, GIT_PATCH_3, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mail-qt1-f174.google.com Received: from mail-qt1-f174.google.com (HELO mail-qt1-f174.google.com) (209.85.160.174) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 24 Jun 2019 23:50:16 +0000 Received: by mail-qt1-f174.google.com with SMTP id d23so1108030qto.2 for ; Mon, 24 Jun 2019 16:50:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=to:from:subject:message-id:date:user-agent:mime-version :content-language; bh=1klIIUUjr1XVLPpwA8YcMr7SxCDaaZrxHrgX9XT+R6Q=; b=sLW6ZtDbk5gzLjKpM3SGMmQt+ggDDq9lMXr25vePXY2R4QUUq+xQ3F2pL3E15c6z67 kblufmoB+NSwApQpChhgUQ464r0GsHZrumcxE/c8CggqqIr1IjaxHk1GinpdpIKVOTvH 4Cce5/ofzUMboFx+g4LoJ6CJZhoFmq6JVbKVsY686NA2dt9ffGavgMhOmui4LysGTy4Q +m2QuxUyTYQzqXymtjB9FPwOX+5LxlOPAWzczVKrj/+7ukh0HL8hJkruIlcZxZourJtg BQxNWW4uzjZiV8vyAL9pTdAmsjfV+lAnSb8ctgdh2zB2T52G2GNmpAlFpTlmYmWlj9XP 7xYw== Received: from [192.168.0.41] (75-166-109-122.hlrn.qwest.net. [75.166.109.122]) by smtp.gmail.com with ESMTPSA id c18sm6669884qkk.73.2019.06.24.16.50.13 for (version=TLS1_3 cipher=AEAD-AES128-GCM-SHA256 bits=128/128); Mon, 24 Jun 2019 16:50:13 -0700 (PDT) To: gcc-patches From: Martin Sebor Subject: [PATCH] constrain one character optimization to one character stores (PR 90989) Message-ID: Date: Mon, 24 Jun 2019 17:50:12 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 X-IsSubscribed: yes The strlen enhancement committed in r263018 to handle multi-character assignments extended the handle_char_store() function to handle such stores via MEM_REFs. Prior to that the function only dealt with single-char stores. The enhancement neglected to constrain a case in the function that assumed the function's previous constraint. As a result, when the original optimization takes place with a multi-character store, the function computes the wrong string length. The attached patch adds the missing constraint. Martin PR tree-optimization - incorrrect strlen result after second strcpy into the same destination gcc/ChangeLog: * tree-ssa-strlen.c (handle_char_store): Constrain a single character optimization to just single character stores. gcc/testsuite/ChangeLog: * gcc.dg/strlenopt-26.c: Exit with test result status. * gcc.dg/strlenopt-67.c: New test. Index: gcc/testsuite/gcc.dg/strlenopt-26.c =================================================================== --- gcc/testsuite/gcc.dg/strlenopt-26.c (revision 272618) +++ gcc/testsuite/gcc.dg/strlenopt-26.c (working copy) @@ -17,8 +17,7 @@ main (void) { char p[] = "foobar"; const char *volatile q = "xyzzy"; - fn1 (p, q); - return 0; + return fn1 (p, q); } /* { dg-final { scan-tree-dump-times "strlen \\(" 2 "strlen" } } */ Index: gcc/testsuite/gcc.dg/strlenopt-67.c =================================================================== --- gcc/testsuite/gcc.dg/strlenopt-67.c (nonexistent) +++ gcc/testsuite/gcc.dg/strlenopt-67.c (working copy) @@ -0,0 +1,52 @@ +/* PR tree-optimization/90989 - incorrrect strlen result after second strcpy + into the same destination. + { dg-do compile } + { dg-options "-O2 -Wall -fdump-tree-optimized" } */ + +// #include "strlenopt.h" + +char a[4]; + +int f4 (void) +{ + char b[4]; + __builtin_strcpy (b, "12"); + + int i = __builtin_strcmp (a, b); + + __builtin_strcpy (b, "123"); + if (__builtin_strlen (b) != 3) + __builtin_abort (); + + return i; +} + +int f6 (void) +{ + char b[6]; + __builtin_strcpy (b, "1234"); + + int i = __builtin_strcmp (a, b); + + __builtin_strcpy (b, "12345"); + if (__builtin_strlen (b) != 5) + __builtin_abort (); + + return i; +} + +int f8 (void) +{ + char b[8]; + __builtin_strcpy (b, "1234"); + + int i = __builtin_strcmp (a, b); + + __builtin_strcpy (b, "1234567"); + if (__builtin_strlen (b) != 7) + __builtin_abort (); + + return i; +} + +/* { dg-final { scan-tree-dump-times "abort|strlen" 0 "optimized" } } */ Index: gcc/tree-ssa-strlen.c =================================================================== --- gcc/tree-ssa-strlen.c (revision 272618) +++ gcc/tree-ssa-strlen.c (working copy) @@ -3462,34 +3462,38 @@ handle_char_store (gimple_stmt_iterator *gsi) return false; } } - /* If si->nonzero_chars > OFFSET, we aren't overwriting '\0', - and if we aren't storing '\0', we know that the length of the - string and any other zero terminated string in memory remains - the same. In that case we move to the next gimple statement and - return to signal the caller that it shouldn't invalidate anything. - This is benefical for cases like: + if (cmp > 0 + && storing_nonzero_p + && TREE_CODE (TREE_TYPE (rhs)) == INTEGER_TYPE) + { + /* Handle a single non-nul character store. + If si->nonzero_chars > OFFSET, we aren't overwriting '\0', + and if we aren't storing '\0', we know that the length of the + string and any other zero terminated string in memory remains + the same. In that case we move to the next gimple statement and + return to signal the caller that it shouldn't invalidate anything. - char p[20]; - void foo (char *q) - { - strcpy (p, "foobar"); - size_t len = strlen (p); // This can be optimized into 6 - size_t len2 = strlen (q); // This has to be computed - p[0] = 'X'; - size_t len3 = strlen (p); // This can be optimized into 6 - size_t len4 = strlen (q); // This can be optimized into len2 - bar (len, len2, len3, len4); - } - */ - else if (storing_nonzero_p && cmp > 0) - { + This is benefical for cases like: + + char p[20]; + void foo (char *q) + { + strcpy (p, "foobar"); + size_t len = strlen (p); // can be folded to 6 + size_t len2 = strlen (q); // has to be computed + p[0] = 'X'; + size_t len3 = strlen (p); // can be folded to 6 + size_t len4 = strlen (q); // can be folded to len2 + bar (len, len2, len3, len4); + } */ gsi_next (gsi); return false; } - else if (storing_all_zeros_p - || storing_nonzero_p - || (offset != 0 && cmp > 0)) + + if (storing_all_zeros_p + || storing_nonzero_p + || (offset != 0 && cmp > 0)) { /* When STORING_NONZERO_P, we know that the string will start with at least OFFSET + 1 nonzero characters. If storing