From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot1-x336.google.com (mail-ot1-x336.google.com [IPv6:2607:f8b0:4864:20::336]) by sourceware.org (Postfix) with ESMTPS id 525643858D28 for ; Mon, 11 Oct 2021 16:23:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 525643858D28 Received: by mail-ot1-x336.google.com with SMTP id u20-20020a9d7214000000b0054e170300adso22282151otj.13 for ; Mon, 11 Oct 2021 09:23:05 -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:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=k1HY3eiL2Z08NYQxCQQDEEs2AzeIwhDfIO4NVBzzXLc=; b=2TVPZpRYRDgTz0dQLM6tBQpDr+YN6it5OowM661pRQ590CTPDObLM8nZZMKbSmXkuU ommIW5yHFqsIWdT8ppmKphqfbESSeABTSHCineeVWi6Uwh1ceqNFGmNNCQjuVe5Lp8W4 YomI6JEhR3PT35Mqa03TVAo8/T5/SVGRp3u8/aihTyYhqs9TlisLe8IDosVdAs/K29w9 CBuwmFjrwnTwy3AaRLTcC+x8pnDDx4PABFwaoupQe/xF5vuA6nsPubQbbmdOxhJoaXxB W2eTuHaKH0F6ipzM48JnAjgVcEnr+s0mNWmNrR1FK3c6/ApegqNZYRYcs3h7Y9spVgMt 44Uw== X-Gm-Message-State: AOAM532c0KQjOUtW3DDkBg7f6WPC50f4jcZ2BWwYphScsuEIfgyHvsaW WDzeajkKVDVLE+8KuMdlTvY= X-Google-Smtp-Source: ABdhPJwkRoHcz+pcFU2T1AgCu3AO0Cro5FsTVZxFxBld++nwPXqh0ELMhVRf2bm+1oNFhswgHcwRPQ== X-Received: by 2002:a9d:7091:: with SMTP id l17mr21463811otj.309.1633969384696; Mon, 11 Oct 2021 09:23:04 -0700 (PDT) Received: from [192.168.0.41] (184-96-250-116.hlrn.qwest.net. [184.96.250.116]) by smtp.gmail.com with ESMTPSA id s10sm1773644oib.58.2021.10.11.09.23.03 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 11 Oct 2021 09:23:04 -0700 (PDT) Subject: Re: [PATCH] rs6000/test: Adjust some cases due to O2 vect [PR102658] To: Segher Boessenkool , "Kewen.Lin" Cc: Bill Schmidt , Hongtao Liu , GCC Patches , David Edelsohn References: <0e964ac9-0e58-33c1-c0ab-24b7f1c60be3@linux.ibm.com> <20211011153050.GV10333@gate.crashing.org> From: Martin Sebor Message-ID: <9bb2743f-cd23-5b7d-6d9d-9917e591377f@gmail.com> Date: Mon, 11 Oct 2021 10:23:03 -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: <20211011153050.GV10333@gate.crashing.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, 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: Mon, 11 Oct 2021 16:23:06 -0000 On 10/11/21 9:30 AM, Segher Boessenkool wrote: > Hi! > > On Mon, Oct 11, 2021 at 10:47:00AM +0800, Kewen.Lin wrote: >> As PR102658 shows, commit r12-4240 enables vectorization at O2, >> some cases need to be adjusted accordingly for rs6000 port. >> >> - For target specific test cases, this adds -fno-tree-vectorize >> to retain original test points, otherwise vectorization can >> make some expected scalar instructions gone or generate some >> unexpected instructions for vector construction. > > Ah good choice. > >> - For generic test cases, it follows the existing suggested >> practice with necessary target/xfail selector. > > Not such a great choice. Many of those tests do not make sense with > vectorisation enabled. This should have been thought about, in some > cases resulting in not running the test with vectorisation enabled, and > in some cases duplicating the test, once with and once without > vectorisation. The tests detect bugs that are present both with and without vetctorization, so they should pass both ways. That they don't tells us that that the warnings need work (they were written with an assumption that doesn't hold anymore). We need to track that work somehow, but simply xfailing them without making a record of what underlying problem the xfails correspond to isn't the best way. In my experience, what works well is opening a bug for each distinct limitation (if one doesn't already exist) and adding a reference to it as a comment to the xfail. > > But you are just following established practice, so :-) > >> - struct A1 a = { 0, { 1 } }; // { dg-warning "\\\[-Wstringop-overflow" "" { target { i?86-*-* x86_64-*-* } } } >> + struct A1 a = { 0, { 1 } }; // { dg-warning "\\\[-Wstringop-overflow" "" { target { i?86-*-* x86_64-*-* powerpc*-*-* } } } As I mentioned in the bug, when adding xfails for regressions please be sure to reference the bug that tracks the underlying root cause. There may be multiple problems, and we need to identify what it is in each instance. As the author of the tests I can help with that but not if I'm not in the loop on these changes (it would seem prudent to get the author's thoughts on such sweeping changes to their work). I discussed one of these failures with Hongtao in detail at the time autovectorization was being enabled and made the same request then but I didn't realize the problem was so pervasive. In addition, the target-specific conditionals in the xfails are going to be difficult to maintain. It might be okay for one or two in a single test but for so many we need a better solution than that. If autovectorization is only enabled for a subset of targets then a solution might be to add a new DejagGNU test for it and conditionalize the xfails on it. Martin > > I don't know if powerpc*-*-* is the correct choice in all these cases. > Sometimes it might have to be powerpc*-*-linux* or similar. We'll find > out :-) > > (An xfail causes XPASS if the test does *not* fail). > >> +/* Now O2 enables vectorization by default, which generates unexpected float >> + conversion for vector construction, so simply disable it. */ > > It is good to see these comments. I love puzzles, but not in the > testsuite! :-) > > Okay for trunk. Thanks! > > > Segher >