[committed] RISC-V: Accept constant operands in la and lla

Palmer Dabbelt palmer@sifive.com
Wed Jun 20 22:53:00 GMT 2018


On Tue, 19 Jun 2018 22:27:25 PDT (-0700), sebastian.huber@embedded-brains.de wrote:
> opcodes/
> 	PR gas/23305
> 	* riscv-opc.c (riscv_opcodes): Use new format specifier 'B' for
> 	la and lla.
>
> gas/
> 	PR gas/23305
> 	* config/tc-riscv.c (riscv_ip): Add format specifier 'B' for
> 	constants and symbols.
> 	* testsuite/gas/riscv/lla32.d: New file.
> 	* testsuite/gas/riscv/lla32.s: Likewise.
> 	* testsuite/gas/riscv/lla64-fail.d: Likewise.
> 	* testsuite/gas/riscv/lla64-fail.l: Likewise.
> 	* testsuite/gas/riscv/lla64-fail.s: Likewise.
> 	* testsuite/gas/riscv/lla64.d: Likewise.
> 	* testsuite/gas/riscv/lla64.s: Likewise.
> ---
>  gas/ChangeLog                        | 13 +++++++++++++
>  gas/config/tc-riscv.c                | 11 +++++++++++
>  gas/testsuite/gas/riscv/lla32.d      | 19 +++++++++++++++++++
>  gas/testsuite/gas/riscv/lla32.s      | 15 +++++++++++++++
>  gas/testsuite/gas/riscv/lla64-fail.d |  3 +++
>  gas/testsuite/gas/riscv/lla64-fail.l |  2 ++
>  gas/testsuite/gas/riscv/lla64-fail.s |  3 +++
>  gas/testsuite/gas/riscv/lla64.d      | 20 ++++++++++++++++++++
>  gas/testsuite/gas/riscv/lla64.s      | 17 +++++++++++++++++
>  opcodes/ChangeLog                    |  6 ++++++
>  opcodes/riscv-opc.c                  |  4 ++--
>  11 files changed, 111 insertions(+), 2 deletions(-)
>  create mode 100644 gas/testsuite/gas/riscv/lla32.d
>  create mode 100644 gas/testsuite/gas/riscv/lla32.s
>  create mode 100644 gas/testsuite/gas/riscv/lla64-fail.d
>  create mode 100644 gas/testsuite/gas/riscv/lla64-fail.l
>  create mode 100644 gas/testsuite/gas/riscv/lla64-fail.s
>  create mode 100644 gas/testsuite/gas/riscv/lla64.d
>  create mode 100644 gas/testsuite/gas/riscv/lla64.s

I'm not opposed to this, but did you check with a RISC-V maintainer before 
committing it?  In general we try to keep our assembly syntax compatible 
between the various assemblers, so any interface addition should be discussed 
to make sure it's feasible everywhere.

This addition shouldn't be a problem, though.

> diff --git a/gas/ChangeLog b/gas/ChangeLog
> index 332a063d55..33ba540ed2 100644
> --- a/gas/ChangeLog
> +++ b/gas/ChangeLog
> @@ -1,3 +1,16 @@
> +2018-06-20  Sebastian Huber  <sebastian.huber@embedded-brains.de>
> +
> +	PR gas/23305
> +	* config/tc-riscv.c (riscv_ip): Add format specifier 'B' for
> +	constants and symbols.
> +	* testsuite/gas/riscv/lla32.d: New file.
> +	* testsuite/gas/riscv/lla32.s: Likewise.
> +	* testsuite/gas/riscv/lla64-fail.d: Likewise.
> +	* testsuite/gas/riscv/lla64-fail.l: Likewise.
> +	* testsuite/gas/riscv/lla64-fail.s: Likewise.
> +	* testsuite/gas/riscv/lla64.d: Likewise.
> +	* testsuite/gas/riscv/lla64.s: Likewise.
> +
>  2018-06-19  Simon Marchi  <simon.marchi@ericsson.com>
>
>  	* Makefile.am (AUTOMAKE_OPTIONS): Remove 1.11, add subdir-objects.
> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
> index a0ea87a3fa..bdec30741b 100644
> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -1935,6 +1935,17 @@ rvc_lui:
>  	      s = expr_end;
>  	      continue;
>
> +	    case 'B':
> +	      my_getExpression (imm_expr, s);
> +	      normalize_constant_expr (imm_expr);
> +	      /* The 'B' format specifier must be a symbol or a constant.  */
> +	      if (imm_expr->X_op != O_symbol && imm_expr->X_op != O_constant)
> +	        break;
> +	      if (imm_expr->X_op == O_symbol)
> +	        *imm_reloc = BFD_RELOC_32;
> +	      s = expr_end;
> +	      continue;
> +
>  	    case 'j': /* Sign-extended immediate.  */
>  	      *imm_reloc = BFD_RELOC_RISCV_LO12_I;
>  	      p = percent_op_itype;
> diff --git a/gas/testsuite/gas/riscv/lla32.d b/gas/testsuite/gas/riscv/lla32.d
> new file mode 100644
> index 0000000000..ab766b4e3b
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/lla32.d
> @@ -0,0 +1,19 @@
> +#as: -march=rv32i -mabi=ilp32
> +#objdump: -dr
> +
> +.*:     file format elf32-littleriscv
> +
> +
> +Disassembly of section .text:
> +
> +0+000 <.text>:
> +   0:	00100513          	li	a0,1
> +   4:	00001537          	lui	a0,0x1
> +   8:	00001537          	lui	a0,0x1
> +   c:	00150513          	addi	a0,a0,1 # 1001 <c>
> +  10:	00001537          	lui	a0,0x1
> +  14:	fff50513          	addi	a0,a0,-1 # fff <d>
> +  18:	80000537          	lui	a0,0x80000
> +  1c:	fff50513          	addi	a0,a0,-1 # 7fffffff <h\+0x80000000>
> +  20:	00000513          	li	a0,0
> +  24:	fff00513          	li	a0,-1
> diff --git a/gas/testsuite/gas/riscv/lla32.s b/gas/testsuite/gas/riscv/lla32.s
> new file mode 100644
> index 0000000000..8d5773e929
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/lla32.s
> @@ -0,0 +1,15 @@
> +.set a, 0x1
> +.set b, 0x1000
> +.set c, 0x1001
> +.set d, 0xfff
> +.set e, 0x7fffffff
> +.set g, 0x0
> +.set h, 0xffffffff
> +.text
> +	lla a0, a
> +	lla a0, b
> +	lla a0, c
> +	lla a0, d
> +	lla a0, e
> +	lla a0, g
> +	lla a0, h
> diff --git a/gas/testsuite/gas/riscv/lla64-fail.d b/gas/testsuite/gas/riscv/lla64-fail.d
> new file mode 100644
> index 0000000000..97b2e105c8
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/lla64-fail.d
> @@ -0,0 +1,3 @@
> +#as: -march=rv64i -mabi=lp64
> +#source: lla64-fail.s
> +#error-output: lla64-fail.l
> diff --git a/gas/testsuite/gas/riscv/lla64-fail.l b/gas/testsuite/gas/riscv/lla64-fail.l
> new file mode 100644
> index 0000000000..088f18e530
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/lla64-fail.l
> @@ -0,0 +1,2 @@
> +.*: Assembler messages:
> +.*: Error: offset too large
> diff --git a/gas/testsuite/gas/riscv/lla64-fail.s b/gas/testsuite/gas/riscv/lla64-fail.s
> new file mode 100644
> index 0000000000..e8379bfbe8
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/lla64-fail.s
> @@ -0,0 +1,3 @@
> +.set a, 0xffffffff
> +.text
> +	lla a0, a

I don't think this should fail on 64-bit systems.  We already have patterns to 
make this work for 'li' so it shouldn't be too much extra work.  As it stands 
it's just odd to have la/lla only support some constants.

> diff --git a/gas/testsuite/gas/riscv/lla64.d b/gas/testsuite/gas/riscv/lla64.d
> new file mode 100644
> index 0000000000..7848eecdfb
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/lla64.d
> @@ -0,0 +1,20 @@
> +#as: -march=rv64i -mabi=lp64
> +#objdump: -dr
> +
> +.*:     file format elf64-littleriscv
> +
> +
> +Disassembly of section .text:
> +
> +0+000 <.text>:
> +   0:	0010051b          	addiw	a0,zero,1
> +   4:	00001537          	lui	a0,0x1
> +   8:	00001537          	lui	a0,0x1
> +   c:	0015051b          	addiw	a0,a0,1
> +  10:	00001537          	lui	a0,0x1
> +  14:	fff5051b          	addiw	a0,a0,-1
> +  18:	80000537          	lui	a0,0x80000
> +  1c:	fff5051b          	addiw	a0,a0,-1
> +  20:	0000051b          	sext.w	a0,zero
> +  24:	fff0051b          	addiw	a0,zero,-1
> +  28:	80000537          	lui	a0,0x80000
> diff --git a/gas/testsuite/gas/riscv/lla64.s b/gas/testsuite/gas/riscv/lla64.s
> new file mode 100644
> index 0000000000..4be5082451
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/lla64.s
> @@ -0,0 +1,17 @@
> +.set a, 0x1
> +.set b, 0x1000
> +.set c, 0x1001
> +.set d, 0xfff
> +.set e, 0x7fffffff
> +.set g, 0x0
> +.set h, 0xffffffffffffffff
> +.set i, 0xffffffff80000000
> +.text
> +	lla a0, a
> +	lla a0, b
> +	lla a0, c
> +	lla a0, d
> +	lla a0, e
> +	lla a0, g
> +	lla a0, h
> +	lla a0, i
> diff --git a/opcodes/ChangeLog b/opcodes/ChangeLog
> index 0cfb8065ee..c5ef6a367e 100644
> --- a/opcodes/ChangeLog
> +++ b/opcodes/ChangeLog
> @@ -1,3 +1,9 @@
> +2018-06-20  Sebastian Huber  <sebastian.huber@embedded-brains.de>
> +
> +	PR gas/23305
> +	* riscv-opc.c (riscv_opcodes): Use new format specifier 'B' for
> +	la and lla.
> +
>  2018-06-19  Simon Marchi  <simon.marchi@ericsson.com>
>
>  	* Makefile.am (AUTOMAKE_OPTIONS): Remove 1.11.
> diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
> index 47e9659c97..8e55916451 100644
> --- a/opcodes/riscv-opc.c
> +++ b/opcodes/riscv-opc.c
> @@ -278,8 +278,8 @@ const struct riscv_opcode riscv_opcodes[] =
>     applied to an add instruction, for relaxation to use.  */
>  {"add",       "I",   "d,s,t,0",MATCH_ADD, MASK_ADD, match_opcode, 0 },
>  {"add",       "I",   "d,s,j",  MATCH_ADDI, MASK_ADDI, match_opcode, INSN_ALIAS },
> -{"la",        "I",   "d,A",  0,    (int) M_LA,  match_never, INSN_MACRO },
> -{"lla",       "I",   "d,A",  0,    (int) M_LLA,  match_never, INSN_MACRO },
> +{"la",        "I",   "d,B",  0,    (int) M_LA,  match_never, INSN_MACRO },
> +{"lla",       "I",   "d,B",  0,    (int) M_LLA,  match_never, INSN_MACRO },
>  {"la.tls.gd", "I",   "d,A",  0,    (int) M_LA_TLS_GD,  match_never, INSN_MACRO },
>  {"la.tls.ie", "I",   "d,A",  0,    (int) M_LA_TLS_IE,  match_never, INSN_MACRO },
>  {"neg",       "I",   "d,t",  MATCH_SUB, MASK_SUB | MASK_RS1, match_opcode, INSN_ALIAS }, /* sub 0 */

Thanks for the patch!



More information about the Binutils mailing list