From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <binutils-return-70606-listarch-binutils=sources.redhat.com@sourceware.org>
Received: (qmail 15427 invoked by alias); 11 Mar 2011 13:26:41 -0000
Received: (qmail 15419 invoked by uid 22791); 11 Mar 2011 13:26:41 -0000
X-SWARE-Spam-Status: No, hits=-1.9 required=5.0	tests=AWL,BAYES_00,T_RP_MATCHES_RCVD
X-Spam-Check-By: sourceware.org
Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100)    by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 11 Mar 2011 13:26:37 +0000
Received: (qmail 21722 invoked from network); 11 Mar 2011 13:26:35 -0000
Received: from unknown (HELO wren.localnet) (paul@127.0.0.2)  by mail.codesourcery.com with ESMTPA; 11 Mar 2011 13:26:35 -0000
From: Paul Brook <paul@codesourcery.com>
To: binutils@sourceware.org
Subject: Re: [PATCH] ARM: gas not detecting invalid operands for Thumb2 ADD{S} and SUB{S}
Date: Fri, 11 Mar 2011 13:26:00 -0000
User-Agent: KMail/1.13.5 (Linux/2.6.37-2-amd64; KDE/4.4.5; x86_64; ; )
Cc: Paul Carroll <pcarroll@codesourcery.com>
References: <4D6BEC05.2010607@codesourcery.com>
In-Reply-To: <4D6BEC05.2010607@codesourcery.com>
MIME-Version: 1.0
Content-Type: Text/Plain;  charset="iso-8859-15"
Content-Transfer-Encoding: 7bit
Message-Id: <201103111326.33651.paul@codesourcery.com>
Mailing-List: contact binutils-help@sourceware.org; run by ezmlm
Precedence: bulk
List-Id: <binutils.sourceware.org>
List-Subscribe: <mailto:binutils-subscribe@sourceware.org>
List-Archive: <http://sourceware.org/ml/binutils/>
List-Post: <mailto:binutils@sourceware.org>
List-Help: <mailto:binutils-help@sourceware.org>, <http://sourceware.org/ml/#faqs>
Sender: binutils-owner@sourceware.org
X-SW-Source: 2011-03/txt/msg00212.txt.bz2

> In ARMv6T2 and ARMv7 Thumb2, the ADD, ADDS, SUB, and SUBS instructions
> added several new instruction forms. One of the new forms allowed is:
> 
>     ADD{S}<c>.W <Rd>,SP,<Rm>{,<shift>}
>     SUB{S}<c>.W <Rd>,SP,<Rm>{,<shift>}

The "added new forms" is mainly an artifact of how the new documentation is 
structured.  If you're starting from the ARM instruction set (or from the 
encodings) then it's actually new restrictions on where r13 (aka SP) may be 
used.

The patch looks ok, though I think the testcases could use some 
reorganisation.  I'd prefer two assembly files, one with insns valid in both 
ARM and Thumb mode, the other which is only valid in ARM mode.  Assemble both 
in both modes.  There should already be existing tests
(e.g. sp-pc-usage-t) that cover the former.

> +@ test case of ADD{S} and SUB{S} instructions in ARM mode

Too vague.  You're testing use of SP in these insns.

> +	.file	"s6163c.c"

Looks bogus.

Paul