From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-vs1-xe31.google.com (mail-vs1-xe31.google.com [IPv6:2607:f8b0:4864:20::e31]) by sourceware.org (Postfix) with ESMTPS id 8B4AA385841A for ; Thu, 23 Sep 2021 01:41:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8B4AA385841A Received: by mail-vs1-xe31.google.com with SMTP id f18so4986897vsp.2 for ; Wed, 22 Sep 2021 18:41:54 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=zyyWtk0Sb5WSdMMruEa8YGs3VsVeR8QChPsUdWb++jU=; b=v7Kdq5ZgnKQCPuag66gYb3j68iHECQSGLHQieJrGX9DFjpABe1Q6Ta8RNvrcQawbP/ FpUpMXjm9T0on0pg0Ilpj53qbQpiQzH2aDZJA8ZP1X/oxU4cZxvzWyAFcWSfEJiLGOMQ cEeIH+jryej8kTkoyVY7KaNDkYDBcXmgY78MyyCkxwde+vHJrZI2WIBbs5Kt4CM6X9Fr 05fbZbHT/p3mCHZW4lEYhg5X9MzFfySPQGLjl5XSPONy+Qe+FifsoEyEcltzJCqkMAd/ pZ/+S9vqHwb3KrBlykIe/9C55BJBw69cBFWe5WARBUoZP9zmgHeed8Io61yedvI8jUN5 nlLQ== X-Gm-Message-State: AOAM5338+13WCEo/WBlrGCDtGGGMjVr+huEYF0ltsDnmko3/BkrtOWiu brKEZ9gGdka72m1mNtWt+wTnQAKRawGwmGZBDAI= X-Google-Smtp-Source: ABdhPJyE/YOl/orw4wNZWd8CeVESOASsKzA4QXs7dF7v7R7+P9pV/M2IuCaPawOSugp3GavDt5LCbtHmTfIlMsmtzPA= X-Received: by 2002:a05:6102:c6:: with SMTP id u6mr2243759vsp.43.1632361313869; Wed, 22 Sep 2021 18:41:53 -0700 (PDT) MIME-Version: 1.0 References: <20210916043305.1674303-1-hongtao.liu@intel.com> <4fe85f3d-b4fd-4419-04f0-c645e23e778d@gmail.com> <5703bdb6-dc01-4436-2403-35832619ebb4@gmail.com> In-Reply-To: <5703bdb6-dc01-4436-2403-35832619ebb4@gmail.com> From: Hongtao Liu Date: Thu, 23 Sep 2021 09:48:12 +0800 Message-ID: Subject: Re: [PATCH] Enable auto-vectorization at O2 with very-cheap cost model. To: Martin Sebor Cc: Richard Biener , Jakub Jelinek , Florian Weimer , Segher Boessenkool , Richard Sandiford , Premachandra.Mallappa@amd.com, GCC Patches , Bill Schmidt , liuhongt , Joseph Myers Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-9.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, 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: Thu, 23 Sep 2021 01:41:56 -0000 On Wed, Sep 22, 2021 at 10:21 PM 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. For the upper case, It points to the second store which is out of bounds, the third store warning is missing. > > >> > >> 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. There are indeed cases of false positives and false negatives .i.e. // Verify warning for access to a definition with an initializer that // initializes the one-element array member. struct A1 a1i_1 = { 0, { 1 } }; void ga1i_1 (void) { a1i_1.a[0] = 0; a1i_1.a[1] = 1; // { dg-warning "\\\[-Wstringop-overflow" } a1i_1.a[2] = 2; // { dg-warning "\\\[-Wstringop-overflow" } struct A1 a = { 0, { 1 } }; --- false positive here. a.a[0] = 1; a.a[1] = 2; // { dg-warning "\\\[-Wstringop-overflow" } false negative here. a.a[2] = 3; // { dg-warning "\\\[-Wstringop-overflow" } false negative here. sink (&a); } the last 2 warnings are missing, and there's new warning on the line *struct A1 a = { 0, { 1 } }; > > Martin -- BR, Hongtao