Re: [PATCH, GAS/ARM] Allow assembly of IT blocks with AL condition

["Richard Earnshaw (lists)" <Richard dot Earnshaw at arm dot com>]

On 22/03/17 11:33, Prakhar Bahuguna wrote:
> According to the ARMv7-M reference manual, section A7.7.37, the IT instruction
> is permitted to accept the AL condition, provided the suffix does not have an
> else. However, the assembler does not allow the AL condition to be specified at
> all. This patch allows instructions to have the AL condition if they are within
> an explicitly defined IT block with the matching condition.
> 
> 2017-02-28  Prakhar Bahuguna  <prakhar.bahuguna@arm.com>
> 
> gas/ChangeLog:
> 
> 	* gas/config/tc-arm.c (it_fsm_pre_encode): Change conditional to first
> 	check for MANUAL_IT_BLOCK state, and update explanatory comment.
> 	* gas/testsuite/gas/arm/arm-it-al.d: New file.
> 	* gas/testsuite/gas/arm/arm-it-al.s: New file.
> 
> Testing done: Ran all gas regression tests, added new test to ensure IT with AL
> condition assembles as expected with instructions outside of IT blocks
> remaining unaffected.
> 
> Okay for master?
> 
> --
> 
> Prakhar Bahuguna
> 
> 
> 0001-Allow-assembly-of-IT-blocks-with-AL-condition.patch
> 
> 
> From 284c841d6866c6d9129a239a79261df3557dba53 Mon Sep 17 00:00:00 2001
> From: Prakhar Bahuguna <prakhar.bahuguna@arm.com>
> Date: Tue, 28 Feb 2017 10:36:54 +0000
> Subject: [PATCH] Allow assembly of IT blocks with AL condition
> 
> ---
>  gas/config/tc-arm.c               |  4 +++-
>  gas/testsuite/gas/arm/arm-it-al.d | 13 +++++++++++++
>  gas/testsuite/gas/arm/arm-it-al.s |  8 ++++++++
>  3 files changed, 24 insertions(+), 1 deletion(-)
>  create mode 100644 gas/testsuite/gas/arm/arm-it-al.d
>  create mode 100644 gas/testsuite/gas/arm/arm-it-al.s
> 
> diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c
> index 60bda51070..a4d2d68e06 100644
> --- a/gas/config/tc-arm.c
> +++ b/gas/config/tc-arm.c
> @@ -18013,6 +18013,8 @@ now_it_add_mask (int cond)
>       1) md_assemble () calls it_fsm_pre_encode () before calling tencode (),
>  	initializing the IT insn type with a generic initial value depending
>  	on the inst.condition.
> +	If the current state is in a manual IT block, the instruction is always
> +	assumed to be an inside insn.
>       2) During the tencode function, two things may happen:
>  	a) The tencode function overrides the IT insn type by
>  	   calling either set_it_insn_type (type) or set_it_insn_type_last ().
> @@ -18058,7 +18060,7 @@ now_it_add_mask (int cond)
>  static void
>  it_fsm_pre_encode (void)
>  {
> -  if (inst.cond != COND_ALWAYS)
> +  if (now_it.state == MANUAL_IT_BLOCK || inst.cond != COND_ALWAYS)
>      inst.it_insn_type = INSIDE_IT_INSN;
>    else
>      inst.it_insn_type = OUTSIDE_IT_INSN;
> diff --git a/gas/testsuite/gas/arm/arm-it-al.d b/gas/testsuite/gas/arm/arm-it-al.d
> new file mode 100644
> index 0000000000..2c4c0da857
> --- /dev/null
> +++ b/gas/testsuite/gas/arm/arm-it-al.d
> @@ -0,0 +1,13 @@
> +#name: ARM IT instruction with AL condition
> +#as: -mthumb -march=armv7a
> +#objdump: -dr --prefix-addresses --show-raw-insn
> +#skip: *-*-*coff *-*-pe *-*-wince *-*-*aout* *-*-netbsd *-*-riscix*
> +
> +.*: +file format .*arm.*
> +
> +
> +Disassembly of section .text:
> +0+0 <.text> bfe4 *	itt	al
> +0+2 <.text\+0x2> 2000 *	moval	r0, #0
> +0+4 <.text\+0x4> 3001 *	addal	r0, #1
> +0+6 <.text\+0x6> f04f 0000 *	mov.w	r0, #0
> diff --git a/gas/testsuite/gas/arm/arm-it-al.s b/gas/testsuite/gas/arm/arm-it-al.s
> new file mode 100644
> index 0000000000..b52aa5aa84
> --- /dev/null
> +++ b/gas/testsuite/gas/arm/arm-it-al.s
> @@ -0,0 +1,8 @@
> +	# Check that IT accepts AL condition
> +	.text
> +	.syntax unified
> +
> +	itt	al
> +	moval	r0, #0
> +	addal	r0, #1
> +	moval	r0, #0
> 


It would also be nice to have an error test that shows that 'ite al'
generates a suitable diagnostic.

Is 'al' mandatory within such a block, or would we accept 'mov	r0, #0'?
(I'd be inclined to say we shouldn't, but that could be debated).

What happens when -mimplicit-it=always is on and I write 'moval r0, #0'
outside of an explicit IT block?  Do I get a normal instruction, or does
it insert an IT instruction before it?

R.