From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 104945 invoked by alias); 7 Oct 2019 14:42:25 -0000 Mailing-List: contact newlib-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: newlib-owner@sourceware.org Received: (qmail 104936 invoked by uid 89); 7 Oct 2019 14:42:25 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-18.2 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.110.172) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 07 Oct 2019 14:42:24 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id B2C03142F; Mon, 7 Oct 2019 07:42:22 -0700 (PDT) Received: from e120077-lin.cambridge.arm.com (e120077-lin.cambridge.arm.com [10.2.206.225]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 419293F6C4; Mon, 7 Oct 2019 07:42:22 -0700 (PDT) Subject: Re: [PATCH] Replace insns in ARMv6-M setjmp impl with flag-setting variants To: Christos Gentsos , newlib@sourceware.org References: <20190930151559.2820173-1-christos.gentsos@cern.ch> From: "Richard Earnshaw (lists)" Message-ID: <20f6d483-602d-28ae-f166-2f7d5e894730@arm.com> Date: Mon, 07 Oct 2019 14:42:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: <20190930151559.2820173-1-christos.gentsos@cern.ch> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2019/txt/msg00600.txt.bz2 On 30/09/2019 16:15, Christos Gentsos wrote: > In the ARMv6-M Thumb instruction set the MOV, ADD and SUB instructions > don't support flag-preserving variants for their 8-bit immediate > forms. > --- > newlib/libc/machine/arm/setjmp.S | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/newlib/libc/machine/arm/setjmp.S b/newlib/libc/machine/arm/setjmp.S > index 1ba711d5..b8511779 100644 > --- a/newlib/libc/machine/arm/setjmp.S > +++ b/newlib/libc/machine/arm/setjmp.S > @@ -74,11 +74,11 @@ SYM (setjmp): > mov r5, sp > mov r6, lr > stmia r0!, {r1, r2, r3, r4, r5, r6} > - sub r0, r0, #40 > + subs r0, r0, #40 > /* Restore callee-saved low regs. */ > ldmia r0!, {r4, r5, r6, r7} > /* Return zero. */ > - mov r0, #0 > + movs r0, #0 > bx lr > > .thumb_func > @@ -86,7 +86,7 @@ SYM (setjmp): > TYPE (longjmp) > SYM (longjmp): > /* Restore High regs. */ > - add r0, r0, #16 > + adds r0, r0, #16 > ldmia r0!, {r2, r3, r4, r5, r6} > mov r8, r2 > mov r9, r3 > @@ -95,12 +95,12 @@ SYM (longjmp): > mov sp, r6 > ldmia r0!, {r3} /* lr */ > /* Restore low regs. */ > - sub r0, r0, #40 > + subs r0, r0, #40 > ldmia r0!, {r4, r5, r6, r7} > /* Return the result argument, or 1 if it is zero. */ > mov r0, r1 > bne 1f > - mov r0, #1 > + movs r0, #1 > 1: > bx r3 > > Sorry for the delay responding to this one, I wanted to look at this in more detail... Having done so, I don't think this is correct. The code you are looking at here is written in legacy syntax, rather than unified syntax. In legacy syntax there was no need to specify the flag-clobbering behaviour of instructions. For example, the longjump code does not make sense otherwise as there is no explicit comparison operation, yet there is a conditional branch near the end: > sub r0, r0, #40 > ldmia r0!, {r4, r5, r6, r7} > /* Return the result argument, or 1 if it is zero. */ > mov r0, r1 // <<<< implicit flag set .... > bne 1f // <<<< ... only makes sense in legacy syntax > mov r0, #1 > 1: How did you build and test this? R.