From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <segher@kernel.crashing.org>
Received: from gate.crashing.org (gate.crashing.org [63.228.1.57])
 by sourceware.org (Postfix) with ESMTP id E25CE3857814
 for <gcc-patches@gcc.gnu.org>; Thu, 22 Apr 2021 22:51:26 +0000 (GMT)
DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org E25CE3857814
Authentication-Results: sourceware.org; dmarc=none (p=none dis=none)
 header.from=kernel.crashing.org
Authentication-Results: sourceware.org;
 spf=fail smtp.mailfrom=segher@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 13MMoQJ1021304;
 Thu, 22 Apr 2021 17:50:26 -0500
Received: (from segher@localhost)
 by gate.crashing.org (8.14.1/8.14.1/Submit) id 13MMoPkN021303;
 Thu, 22 Apr 2021 17:50:25 -0500
X-Authentication-Warning: gate.crashing.org: segher set sender to
 segher@kernel.crashing.org using -f
Date: Thu, 22 Apr 2021 17:50:25 -0500
From: Segher Boessenkool <segher@kernel.crashing.org>
To: will schmidt <will_schmidt@vnet.ibm.com>
Cc: Michael Meissner <meissner@linux.ibm.com>, gcc-patches@gcc.gnu.org,
 David Edelsohn <dje.gcc@gmail.com>, Bill Schmidt <wschmidt@linux.ibm.com>,
 Peter Bergner <bergner@linux.ibm.com>
Subject: Re: [PATCH] Fix logic error in 32-bit trampolines, PR target/98952
Message-ID: <20210422225025.GG27473@gate.crashing.org>
References: <20210409210907.GA5325@ibm-toto.the-meissners.org>
 <3eaf3e20295b70a9f5d894931dae0d67caccd69b.camel@vnet.ibm.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <3eaf3e20295b70a9f5d894931dae0d67caccd69b.camel@vnet.ibm.com>
User-Agent: Mutt/1.4.2.3i
X-Spam-Status: No, score=-6.4 required=5.0 tests=BAYES_00, JMQ_SPF_NEUTRAL,
 KAM_DMARC_STATUS, KAM_NUMSUBJECT, TXREP, T_SPF_HELO_PERMERROR,
 T_SPF_PERMERROR autolearn=no autolearn_force=no version=3.4.2
X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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 <gcc-patches.gcc.gnu.org>
List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>,
 <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe>
List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/>
List-Post: <mailto:gcc-patches@gcc.gnu.org>
List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help>
List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>,
 <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe>
X-List-Received-Date: Thu, 22 Apr 2021 22:51:28 -0000

On Mon, Apr 12, 2021 at 05:02:38PM -0500, will schmidt wrote:
> On Fri, 2021-04-09 at 17:09 -0400, Michael Meissner wrote:
> > -	li	r8,trampoline_size	/* verify that the trampoline is big enough */
> > -	cmpw	cr1,r8,r4
> > +	cmpwi	cr1,r4,trampoline_size	/* verify that the trampoline is big enough */
> 
> Hmm, I spent several minutes trying to determine how cmpw behaves
> differently than cmpwi before noticing you also swapped the
> order of the r4,r8 operands. 
> 
> That seems OK.
> 
> A statement in the description indicating that you used a cmpwi instead
> of a cmpw since you were in the neighborhood would help call that out. 

In general, don't do two unrelated things, esp. when not pointing it out
explicitly.

> The #elif  _CALL_ELF == 2  portion of tramp.S (line 159 or so) has a
> similar compare stanza with respect to the order of operands on the
> compare.  Will this also have a backwards greater-than less-than issue?
> 
> 	li	r8,trampoline_size	/* verify that the trampoline is big enough */
> 	cmpw	cr1,r8,r4
> 	srwi	r4,r4,3		/* # doublewords to move */
> 	addi	r9,r3,-8	/* adjust pointer for stdu */
> 	mtctr	r4
> 	blt	cr1,.Labort

Not sure...  It should use cmpwi as well though, and then it is easier
to see.


Segher