This is the mail archive of the binutils@sources.redhat.com mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Thumb32 assembler (11,58/69)


Paul Brook <paul@codesourcery.com> writes:
>
>         .macro popret regs
>         ldmia sp!, {\regs, pc}
>         .endm
>         .text
>         popret "r4, r5"
>
> foo.s: Assembler messages:
> foo.s:5: Error: ARM register expected -- `ldmia sp!,{r4, r5,pc}'

This turns out to be a bug in the generic macro code.  do_scrub_chars
needs to be applied to the result of macro expansion, because macro
expansion can generate lines that are not in canonical form.

Richard Earnshaw <rearnsha@gcc.gnu.org> writes:
> On Tue, 2005-04-26 at 17:32, Paul Brook wrote:
>> Unfortunately gcc (incorrectly) outputs the three argument form
>>     mul rd, rd, rm
>> I'll fix gcc, but I guess we want to support this for backwards
>> compatibility.
>
> The ARMv6 ARM says that all known implementations of v4t and later can
> handle any register overlap for MUL, so I'm inclined to just relax the
> compiler and assembler constraints entirely.

Easily enough done.

The appended patch applies on top of the previous 69 patches; it fixes
the generic macro expansion bug, allows either source argument of
Thumb mul to equal the destination, and fixes a typo in the handling
of other commutative Thumb arithmetic instructions.  Dependencies on
as.h do not appear to be explicitly recorded in the Makefile, so
adding an #include of as.h to sb.c does not require a Makefile
update.  (Incidentally, "make DEP" doesn't work for me - DEPA contains
zillions of absolute paths, despite dep.sed supposedly filtering them.)

I am still working on the problem with 'ldr rd, label' at an
unaligned-modulo-4 address.  I thought I could use fx_pcrel to
distinguish between that and 'ldr rd, [pc, #4]', but the generic fixup
logic clears fx_pcrel, so another approach is necessary.

zw

gas:
        * sb.c: Include as.h.
        (sb_to_scrub, scrub_position, scrub_from_sb): New statics.
        (sb_scrub_and_add_sb): New interface.
        * sb.h: Declare sb_scrub_and_add_sb.
        * input-scrub.c (input_scrub_include_sb): Use it.

        * config/tc-arm.c (do_t_arit3c): Correct typo in expression.
        (do_t_mul): Allow dest to equal either source1 or source2 in
        16-bit form; do not complain about dest == source1 in any
        case.

gas/testsuite:
        * gas/arm/tcompat2.s: Test both dest==source1 and dest==source2 
        for commutative arithmetic instructions.
        * gas/arm/tcompat2.d: Update to match.
        * gas/arm/t16-bad.l: Adjust expected diagnostic.
        * gas/arm/macro1.s, gas/arm/macro1.d: New test pair.
        * gas/arm/arm.exp: Run it.

===================================================================
Index: gas/sb.c
--- gas/sb.c	(revision 84)
+++ gas/sb.c	(working copy)
@@ -33,6 +33,7 @@
 #endif
 #include "libiberty.h"
 #include "sb.h"
+#include "as.h"
 
 /* These routines are about manipulating strings.
 
@@ -121,6 +122,37 @@ sb_add_sb (sb *ptr, sb *s)
   ptr->len += s->len;
 }
 
+/* helper for below */
+static sb *sb_to_scrub;
+static char *scrub_position;
+static int
+scrub_from_sb (char *buf, int buflen)
+{
+  int copy;
+  copy = sb_to_scrub->len - (scrub_position - sb_to_scrub->ptr);
+  if (copy > buflen)
+    copy = buflen;
+  memcpy (buf, scrub_position, copy);
+  scrub_position += copy;
+  return copy;
+}
+
+/* run the sb at s through do_scrub_chars and add the result to the sb
+   at ptr */
+
+void
+sb_scrub_and_add_sb (sb *ptr, sb *s)
+{
+  sb_to_scrub = s;
+  scrub_position = s->ptr;
+  
+  sb_check (ptr, s->len);
+  ptr->len += do_scrub_chars (scrub_from_sb, ptr->ptr + ptr->len, s->len);
+
+  sb_to_scrub = 0;
+  scrub_position = 0;
+}
+
 /* make sure that the sb at ptr has room for another len characters,
    and grow it if it doesn't.  */
 
===================================================================
Index: gas/sb.h
--- gas/sb.h	(revision 84)
+++ gas/sb.h	(working copy)
@@ -82,6 +82,7 @@ extern void sb_build (sb *, int);
 extern void sb_new (sb *);
 extern void sb_kill (sb *);
 extern void sb_add_sb (sb *, sb *);
+extern void sb_scrub_and_add_sb (sb *, sb *);
 extern void sb_reset (sb *);
 extern void sb_add_char (sb *, int);
 extern void sb_add_string (sb *, const char *);
===================================================================
Index: gas/input-scrub.c
--- gas/input-scrub.c	(revision 84)
+++ gas/input-scrub.c	(working copy)
@@ -280,7 +280,7 @@ input_scrub_include_sb (sb *from, char *
       /* Add the sentinel required by read.c.  */
       sb_add_char (&from_sb, '\n');
     }
-  sb_add_sb (&from_sb, from);
+  sb_scrub_and_add_sb (&from_sb, from);
   sb_index = 1;
 
   /* These variables are reset by input_scrub_push.  Restore them
===================================================================
Index: gas/config/tc-arm.c
--- gas/config/tc-arm.c	(revision 84)
+++ gas/config/tc-arm.c	(working copy)
@@ -6025,7 +6025,7 @@ do_t_arit3c (void)
       if (Rd == Rs)
 	inst.instruction |= Rn << 3;
       else if (Rd == Rn)
-	inst.instruction |= Rn << 3;
+	inst.instruction |= Rs << 3;
       else
 	constraint (1, _("dest must overlap one source register"));
     }
@@ -6735,15 +6735,18 @@ do_t_mul (void)
     {
       constraint (!thumb32_mode
 		  && inst.instruction == T_MNEM_muls, BAD_THUMB32);
-      constraint (inst.operands[0].reg > 7 || inst.operands[1].reg > 7, BAD_HIREG);
-      constraint (inst.operands[0].reg != inst.operands[2].reg,
-		  _("dest and source2 must be the same register"));
-      if (inst.operands[0].reg == inst.operands[1].reg)
-	as_tsktsk (_("dest and source must be different in MUL"));
+      constraint (inst.operands[0].reg > 7 || inst.operands[1].reg > 7,
+		  BAD_HIREG);
 
       inst.instruction = THUMB_OP16 (inst.instruction);
       inst.instruction |= inst.operands[0].reg;
-      inst.instruction |= inst.operands[1].reg << 3;
+
+      if (inst.operands[0].reg == inst.operands[1].reg)
+	inst.instruction |= inst.operands[2].reg << 3;
+      else if (inst.operands[0].reg == inst.operands[2].reg)
+	inst.instruction |= inst.operands[1].reg << 3;
+      else
+	constraint (1, _("dest must overlap one source register"));
     }
 }
 
===================================================================
Index: gas/testsuite/gas/arm/tcompat2.s
--- gas/testsuite/gas/arm/tcompat2.s	(revision 84)
+++ gas/testsuite/gas/arm/tcompat2.s	(working copy)
@@ -1,13 +1,27 @@
 	@ Three-argument forms of Thumb arithmetic instructions.
+	@ Commutative instructions allow either the second or third
+	@ operand to equal the first.
+
 	.text
 	.global m
 	.thumb_func
 m:
 	adc	r0,r0,r1
+	adc	r0,r1,r0
+
 	and	r0,r0,r1
-	bic	r0,r0,r1
+	and	r0,r1,r0
+
 	eor	r0,r0,r1
+	eor	r0,r1,r0
+
+	mul	r0,r0,r1
 	mul	r0,r1,r0
+
 	orr	r0,r0,r1
+	orr	r0,r1,r0
+
+	bic	r0,r0,r1
+
 	sbc	r0,r0,r1
-	nop
+
===================================================================
Index: gas/testsuite/gas/arm/tcompat2.d
--- gas/testsuite/gas/arm/tcompat2.d	(revision 84)
+++ gas/testsuite/gas/arm/tcompat2.d	(working copy)
@@ -9,10 +9,14 @@
 Disassembly of section .text:
 
 0+00 <[^>]*> 4148 *	adcs	r0, r1
-0+02 <[^>]*> 4008 *	ands	r0, r1
-0+04 <[^>]*> 4388 *	bics	r0, r1
-0+06 <[^>]*> 4048 *	eors	r0, r1
-0+08 <[^>]*> 4348 *	muls	r0, r1
-0+0a <[^>]*> 4308 *	orrs	r0, r1
-0+0c <[^>]*> 4188 *	sbcs	r0, r1
-0+0e <[^>]*> 46c0 *	nop			\(mov r8, r8\)
+0+02 <[^>]*> 4148 *	adcs	r0, r1
+0+04 <[^>]*> 4008 *	ands	r0, r1
+0+06 <[^>]*> 4008 *	ands	r0, r1
+0+08 <[^>]*> 4048 *	eors	r0, r1
+0+0a <[^>]*> 4048 *	eors	r0, r1
+0+0c <[^>]*> 4348 *	muls	r0, r1
+0+0e <[^>]*> 4348 *	muls	r0, r1
+0+10 <[^>]*> 4308 *	orrs	r0, r1
+0+12 <[^>]*> 4308 *	orrs	r0, r1
+0+14 <[^>]*> 4388 *	bics	r0, r1
+0+16 <[^>]*> 4188 *	sbcs	r0, r1
===================================================================
Index: gas/testsuite/gas/arm/t16-bad.l
--- gas/testsuite/gas/arm/t16-bad.l	(revision 84)
+++ gas/testsuite/gas/arm/t16-bad.l	(working copy)
@@ -70,7 +70,7 @@
 [^:]*:53: Error: unshifted register required -- `sbc r0,#12'
 [^:]*:53: Error: unshifted register required -- `sbc r0,r1,lsl#2'
 [^:]*:53: Error: unshifted register required -- `sbc r0,r1,lsl r3'
-[^:]*:54: Error: dest and source2 must be the same register -- `mul r1,r2,r3'
+[^:]*:54: Error: dest must overlap one source register -- `mul r1,r2,r3'
 [^:]*:54: Error: lo register required -- `mul r8,r0'
 [^:]*:54: Error: lo register required -- `mul r0,r8'
 [^:]*:62: Error: lo register required -- `asr r8,r0,#12'
===================================================================
Index: gas/testsuite/gas/arm/macro1.d
--- gas/testsuite/gas/arm/macro1.d	(revision 0)
+++ gas/testsuite/gas/arm/macro1.d	(revision 0)
@@ -0,0 +1,9 @@
+# name: Macro scrubbing
+# as:
+# objdump: -dr --prefix-addresses --show-raw-insn
+
+[^:]+: +file format .*arm.*
+
+Disassembly of section .text:
+
+0+0 <[^>]*> e8bd8030 ?	ldmia	sp!, {r4, r5, pc}
===================================================================
Index: gas/testsuite/gas/arm/macro1.s
--- gas/testsuite/gas/arm/macro1.s	(revision 0)
+++ gas/testsuite/gas/arm/macro1.s	(revision 0)
@@ -0,0 +1,6 @@
+	@ Test that macro expansions are properly scrubbed.
+	.macro popret regs
+	ldmia sp!, {\regs, pc}
+	.endm
+	.text
+	popret "r4, r5"
===================================================================
Index: gas/testsuite/gas/arm/arm.exp
--- gas/testsuite/gas/arm/arm.exp	(revision 84)
+++ gas/testsuite/gas/arm/arm.exp	(working copy)
@@ -55,6 +55,7 @@ if {[istarget *arm*-*-*] || [istarget "x
     run_dump_test "tcompat"
     run_dump_test "tcompat2"
     run_dump_test "iwmmxt"
+    run_dump_test "macro1"
     
     run_errors_test "vfp-bad" "-mfpu=vfp" "VFP errors"
     run_errors_test "req" "-mcpu=arm7m" ".req errors"


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]