From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate.crashing.org (gate.crashing.org [63.228.1.57]) by sourceware.org (Postfix) with ESMTP id 3EC073858289; Thu, 25 May 2023 15:36:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 3EC073858289 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=kernel.crashing.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=kernel.crashing.org Received: from gate.crashing.org (localhost.localdomain [127.0.0.1]) by gate.crashing.org (8.14.1/8.14.1) with ESMTP id 34PFXZaZ006043; Thu, 25 May 2023 10:33:35 -0500 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id 34PFXX8o006042; Thu, 25 May 2023 10:33:33 -0500 X-Authentication-Warning: gate.crashing.org: segher set sender to segher@kernel.crashing.org using -f Date: Thu, 25 May 2023 10:33:32 -0500 From: Segher Boessenkool To: Alexandre Oliva Cc: "Kewen.Lin" , Rainer Orth , Mike Stump , David Edelsohn , Kewen Lin , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] [testsuite] [powerpc] adjust -m32 counts for fold-vec-extract* Message-ID: <20230525153332.GK19790@gate.crashing.org> References: <0737fbfc-726c-ffca-5f36-d6b3f0decfec@linux.ibm.com> <20230525112200.GJ19790@gate.crashing.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.3i X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,JMQ_SPF_NEUTRAL,KAM_DMARC_STATUS,SPF_HELO_PASS,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi Alex, On Thu, May 25, 2023 at 10:55:37AM -0300, Alexandre Oliva wrote: > On May 25, 2023, Segher Boessenkool wrote: > > Fwiw, updating the insn counts blindly like this > > ... is a claim that carries a wildly incorrect and insulting underlying > assumption: Sorry you feel that way. I'm not even assuming anything :-( > I've actually identified the corresponding change to the > lp64 tests, compared the effects of the codegen changes, and concluded > the tests needed this changing for ilp32 to keep on testing for the same > thing after code changes brought about by changes that AFAICT had been > well understood when making the lp64 adjustments. But you didn't explain any of that (saying it is so is not the same thing at all as explaining it!) > > If it is not possible to keep these tests up-to-date easily > > The counts have been stable for a couple of release cycles already. > > The change that caused the codegen differences is identified and > understood; the PR confirmed my findings, naming the root cause and the > incomplete testsuite adjustment. Oh, was this discussed in some PR? The patch submission should have carried the conclusions from the discussions there then :-) > I suspect there may also be ABI-related assumptions implied by the 'add' > counts, but I don't know enough about all the ppc variants to be sure. The compiler can and will create all kinds of code for wildly unexpected reasons. "add" is dangerous to count already, but it is not as bad as "addi" :-) > Now, if your implied claim is correct that counting 'add/addi' > instructions in these tests is fragile, dropping the checks for those > would probably be best. The same is true for almost all instructions. You can only sanely count instructions if either you count only unusual insns, or if you test only *tiny* functions (say five insns, including the blr at the end!) > But if ppc maintainers seem to have different > opinions as to how to deal with the fallout of that one-time codegen > change, it would be foolish for me to get pulled into the cross fire. There is no crossfire. I did not dis-approve the patch, just said this is a high maintenance direction to proceed in. There has been a lot of that the last few years, we should improve on that. It is not about this patch (only). > Here's the patch that corrects the long-broken counts, with the > requested adjustments, retested with ppc- and ppc64-vx7r2. Ok? > Codegen changes caused add instruction count mismatches on > ppc-*-linux-gnu and other 32-bit ppc targets. At some point the > expected counts were adjusted for lp64, but ilp32 differences > remained, and published test results confirm it. ... and this is not something that can be confimed like this. Just spend a few minutes more to put *actual numbers* here, with some indication this is good and correct codegen, so that it is bloody easy for a reviewer to review and for a maintainer to approve! > /* -m32 target has an 'add' in place of one of the 'addi'. */ > -/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 2 { target lp64 } } } */ > -/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 3 { target ilp32 } } } */ > +/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 2 } } */ Just {\madd} or more conservative {\maddi?\M} then? Segher