From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot1-x32c.google.com (mail-ot1-x32c.google.com [IPv6:2607:f8b0:4864:20::32c]) by sourceware.org (Postfix) with ESMTPS id 697B4385841B for ; Wed, 22 Sep 2021 15:03:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 697B4385841B Received: by mail-ot1-x32c.google.com with SMTP id o59-20020a9d2241000000b0054745f28c69so1795989ota.13 for ; Wed, 22 Sep 2021 08:03:23 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:subject:from:to:cc:references:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=mlvyAfPw3Crt3OHotUtpgqZyXzIRsv7NzRoejQ2aoOU=; b=V6jU932ZnRcJBdEF5j9ZKR1c7wnfP6rMZNSE/kIVuEY5HAOok5bmzNAADwtNLgGSmq p2JzPG2NJE2DYYpgiYUsM7KqDrMRa7w97Gfdt+g2hXsL8KvwjSO7NnnWmWWAQT7LQDek tuY7xHwKv+VQAd1Vzs2SkMyZ4c2bLmmy9UzO7zxLOs3bLmRHLG/EzP2Y4pudWsIZWuOk 0GWOAjmNxsrdshb8zggK0EN+DnhpgpixIHndg9l5vVG9aa4ItOEL3yzt9h2ZtGKtLSP9 hSfFLup//q94IUc3s4KR0VyKdufzPDRObqVlCiZBFi+e61E41mzf48+vDPVsq4AFs0h1 Gc4w== X-Gm-Message-State: AOAM530HDaTnPFNofjYqSK3c2kTtm37MZZCmOvuJlF650meN1MqpyyAf X30xjLLUOWYHGLEAm2h0HW0= X-Google-Smtp-Source: ABdhPJx2hUilacL/w5CmhUtKlCARlErOlH2UxtXzQE1Dk16Yp97rg13U1CNiiNhByA8tM6xEp0c9QQ== X-Received: by 2002:a05:6830:2685:: with SMTP id l5mr228330otu.9.1632323002757; Wed, 22 Sep 2021 08:03:22 -0700 (PDT) Received: from [192.168.0.41] (97-118-96-133.hlrn.qwest.net. [97.118.96.133]) by smtp.gmail.com with ESMTPSA id 9sm554344oir.10.2021.09.22.08.03.21 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 22 Sep 2021 08:03:22 -0700 (PDT) Subject: Re: [PATCH] Enable auto-vectorization at O2 with very-cheap cost model. From: Martin Sebor To: Hongtao Liu Cc: Richard Biener , Jakub Jelinek , Florian Weimer , Segher Boessenkool , Richard Sandiford , Premachandra.Mallappa@amd.com, GCC Patches , Bill Schmidt , liuhongt , Joseph Myers References: <20210916043305.1674303-1-hongtao.liu@intel.com> <4fe85f3d-b4fd-4419-04f0-c645e23e778d@gmail.com> <5703bdb6-dc01-4436-2403-35832619ebb4@gmail.com> Message-ID: Date: Wed, 22 Sep 2021 09:03:20 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 In-Reply-To: <5703bdb6-dc01-4436-2403-35832619ebb4@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-8.6 required=5.0 tests=BAYES_00, BODY_8BITS, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 22 Sep 2021 15:03:24 -0000 On 9/22/21 8:21 AM, Martin Sebor wrote: > On 9/21/21 7:38 PM, Hongtao Liu wrote: >> On Mon, Sep 20, 2021 at 4:13 AM Martin Sebor wrote: > ... >>>>>> diff --git a/gcc/testsuite/c-c++-common/Wstringop-overflow-2.c >>>>>> b/gcc/testsuite/c-c++-common/Wstringop-overflow-2.c >>>>>> index 1d79930cd58..9351f7e7a1a 100644 >>>>>> --- a/gcc/testsuite/c-c++-common/Wstringop-overflow-2.c >>>>>> +++ b/gcc/testsuite/c-c++-common/Wstringop-overflow-2.c >>>>>> @@ -1,7 +1,7 @@ >>>>>>    /* PR middle-end/91458 - inconsistent warning for writing past >>>>>> the end >>>>>>       of an array member >>>>>>       { dg-do compile } >>>>>> -   { dg-options "-O2 -Wall -Wno-array-bounds -fno-ipa-icf" } */ >>>>>> +   { dg-options "-O2 -Wall -Wno-array-bounds -fno-ipa-icf >>>>>> -fno-tree-vectorize" } */ >>>>> >>>>> The testcase is large - what part requires this change?  Given the >>>>> testcase was added for inconsistent warnings do they now become >>>>> inconsistent again as we enable vectorization at -O2? >>>>> >>>>> That said, the testcase adjustments need some explaining - I suppose >>>>> you didn't just slap -fno-tree-vectorize to all of those changing >>>>> behavior? >>>>> >>>> void ga1_ (void) >>>> { >>>>     a1_.a[0] = 0; >>>>     a1_.a[1] = 1;                 // { dg-warning >>>> "\\\[-Wstringop-overflow" } >>>>     a1_.a[2] = 2;                 // { dg-warning >>>> "\\\[-Wstringop-overflow" } >>>> >>>>     struct A1 a; >>>>     a.a[0] = 0; >>>>     a.a[1] = 1;                   // { dg-warning >>>> "\\\[-Wstringop-overflow" } >>>>     a.a[2] = 2;                   // { dg-warning >>>> "\\\[-Wstringop-overflow" } >>>>     sink (&a); >>>> } >>>> >>>> It's supposed to be 2 warning for a.a[1] = 1 and a.a[2] = 1 since >>>> there are 2 accesses, but after enabling vectorization, there's only >>>> one access, so one warning is missing which causes the failure. > > With the stores vectorized, is the warning on the correct line or > does it point to the first store, the one that's in bounds, as > it does with -O3?  The latter would be a regression at -O2. > >>> >>> I would find it preferable to change the test code over disabling >>> optimizations that are on by default.  My concern is that the test >>> would no longer exercise the default behavior.  (The same goes for >>> the -fno-ipa-icf option.) >> Hmm, it's a middle-end test, for some backend, it may not do >> vectorization(it depends on TARGET_VECTOR_MODE_SUPPORTED_P and >> relative cost model). > > Yes, there are quite a few warning tests like that.  Their main > purpose is to verify that in common GCC invocations (i.e., without > any special options) warnings are a) issued when expected and b) > not issued when not expected.  Otherwise, middle end warnings are > known to have both false positives and false negatives in some > invocations, depending on what optimizations are in effect. > Indiscriminately disabling common optimizations for these large > tests and invoking them under artificial conditions would > compromise this goal and hide the problems. > > If enabling vectorization at -O2 causes regressions in the quality > of diagnostics (as the test failure above indicates seems to be > happening) we should investigate these and open bugs for them so > they can be fixed.  We can then tweak the specific failing test > cases to avoid the failures until they are fixed. To expand on the last part: in my tests with -O2 and -O3 the failure is specific to char stores and doesn't affect stores of larger types because they're not detected by -Wstringop-overflow. Those accesses are handled by -Warray-bounds. The former runs as part of the strlen and after vectorization, while the latter runs in vrp1 and before vectorization. So the "quick and dirty" solution here, to keep the warnings on the right lines, might be to move the char store handling from the strlen pass to vrp1. A more robust solution is to avoid vectorizing (or merging) out of bounds stores. > > Martin