This is the mail archive of the
binutils@sources.redhat.com
mailing list for the binutils project.
Re: GAS for ARM: Reject ASR/LSR/ROR with immediate of 0
- To: rearnsha at arm dot com
- Subject: Re: GAS for ARM: Reject ASR/LSR/ROR with immediate of 0
- From: Nick Clifton <nickc at redhat dot com>
- Date: Wed, 16 Aug 2000 12:00:06 -0700
- CC: scottb at netwinder dot org, phillpB at cygnus dot com, binutils at sources dot redhat dot com
Hi Richard,
: Immediately below your proposed fix is a piece of code that fixes this up,
: so that shifts of zero are converted into lsl shifts. So the assembler
: should never generate code that doesn't do as the coder expected (in
: particular, you don't get an unexpected shift of 32). If your patch goes
: in, then that should be removed.
:
: I guess it's a matter of taste as to which should be done. Maybe it would
: be better to just emit some sort of warning and continue as before. Is
: there some particular reason why this has come up?
:
: On a final matter of style, I think we should add some defines for the the
: shift bit patterns. Comparing shft->value with 0 is not particularly
: intuitive.
: Comparing it with SHFT_ASL would be much clearer.
Agreed. In fact I decided to tidy up that whole code area (see the
patch below). One thing I did add was a warning message if "ROR #0"
is used, since this is turned into RRX, which may not have been what
the programmer was expecting.
If anyone has any problems with this patch (which I am about to check
in), please let me know.
Cheers
Nick
gas/ChangeLog
2000-08-16 Nick Clifton <nickc@redhat.com>
* config/tc-arm.c (struct asm_shift): Delete.
(shift[]): Delete.
(enum asm_shift_index): New.
(struct asm_shift_properties): New.
(struct asm_shift_name): New.
(shift_properties[]); New.
(shift_names[]); New.
(decode_shift): Use new structures.
Issue a warning is "ROR #0" is used.
Issue a warning if "ASR #0" or "LSR #0" is used.
(md_begin): Initialise arm_shift_hsh table from new
asm_shift_name array.
gas/testsuite/ChangeLog
2000-08-16 Nick Clifton <nickc@redhat.com>
* gas/arm/inst.s: Add tests for edge cases of shift based
addressing modes.
* gas/arm/inst.d: Add expected results for new tests.
Index: gas/config/tc-arm.c
===================================================================
RCS file: /cvs/src//src/gas/config/tc-arm.c,v
retrieving revision 1.54
diff -p -r1.54 tc-arm.c
*** tc-arm.c 2000/08/16 17:48:50 1.54
--- tc-arm.c 2000/08/16 18:57:55
*************** struct arm_it
*** 166,193 ****
struct arm_it inst;
! struct asm_shift
{
! CONST char * template;
! unsigned long value;
};
! static CONST struct asm_shift shift[] =
{
! {"asl", 0},
! {"lsl", 0},
! {"lsr", 0x00000020},
! {"asr", 0x00000040},
! {"ror", 0x00000060},
! {"rrx", 0x00000060},
! {"ASL", 0},
! {"LSL", 0},
! {"LSR", 0x00000020},
! {"ASR", 0x00000040},
! {"ROR", 0x00000060},
! {"RRX", 0x00000060}
};
#define NO_SHIFT_RESTRICT 1
#define SHIFT_RESTRICT 0
--- 166,219 ----
struct arm_it inst;
! enum asm_shift_index
{
! SHIFT_LSL = 0,
! SHIFT_LSR,
! SHIFT_ASR,
! SHIFT_ROR,
! SHIFT_RRX
! };
!
! struct asm_shift_properties
! {
! enum asm_shift_index index;
! unsigned long bit_field;
! unsigned int allows_0 : 1;
! unsigned int allows_32 : 1;
};
! static const struct asm_shift_properties shift_properties [] =
{
! { SHIFT_LSL, 0, 1, 0},
! { SHIFT_LSR, 0x20, 0, 1},
! { SHIFT_ASR, 0x40, 0, 1},
! { SHIFT_ROR, 0x60, 0, 0},
! { SHIFT_RRX, 0x60, 0, 0}
};
+ struct asm_shift_name
+ {
+ const char * name;
+ const struct asm_shift_properties * properties;
+ };
+
+ static const struct asm_shift_name shift_names [] =
+ {
+ { "asl", shift_properties + SHIFT_LSL },
+ { "lsl", shift_properties + SHIFT_LSL },
+ { "lsr", shift_properties + SHIFT_LSR },
+ { "asr", shift_properties + SHIFT_ASR },
+ { "ror", shift_properties + SHIFT_ROR },
+ { "rrx", shift_properties + SHIFT_RRX },
+ { "ASL", shift_properties + SHIFT_LSL },
+ { "LSL", shift_properties + SHIFT_LSL },
+ { "LSR", shift_properties + SHIFT_LSR },
+ { "ASR", shift_properties + SHIFT_ASR },
+ { "ROR", shift_properties + SHIFT_ROR },
+ { "RRX", shift_properties + SHIFT_RRX }
+ };
+
#define NO_SHIFT_RESTRICT 1
#define SHIFT_RESTRICT 0
*************** decode_shift (str, unrestrict)
*** 2505,2511 ****
char ** str;
int unrestrict;
{
! struct asm_shift * shft;
char * p;
char c;
--- 2531,2537 ----
char ** str;
int unrestrict;
{
! struct asm_shift_name * shift;
char * p;
char c;
*************** decode_shift (str, unrestrict)
*** 2522,2604 ****
c = * p;
* p = '\0';
! shft = (struct asm_shift *) hash_find (arm_shift_hsh, * str);
* p = c;
! if (shft)
{
! if ( ! strncmp (* str, "rrx", 3)
! || ! strncmp (* str, "RRX", 3))
! {
! * str = p;
! inst.instruction |= shft->value;
! return SUCCESS;
! }
! skip_whitespace (p);
! if (unrestrict && reg_required_here (& p, 8) != FAIL)
! {
! inst.instruction |= shft->value | SHIFT_BY_REG;
! * str = p;
! return SUCCESS;
! }
! else if (is_immediate_prefix (* p))
! {
! inst.error = NULL;
! p ++;
! if (my_get_expression (& inst.reloc.exp, & p))
! return FAIL;
! /* Validate some simple #expressions. */
! if (inst.reloc.exp.X_op == O_constant)
{
! unsigned num = inst.reloc.exp.X_add_number;
!
! /* Reject operations greater than 32, or lsl #32. */
! if (num > 32 || (num == 32 && shft->value == 0))
! {
! inst.error = _("Invalid immediate shift");
! return FAIL;
! }
!
! /* Shifts of zero should be converted to lsl
! (which is zero). */
! if (num == 0)
! {
! * str = p;
! return SUCCESS;
! }
!
! /* Shifts of 32 are encoded as 0, for those shifts that
! support it. */
! if (num == 32)
! num = 0;
!
! inst.instruction |= (num << 7) | shft->value;
! * str = p;
! return SUCCESS;
}
-
- inst.reloc.type = BFD_RELOC_ARM_SHIFT_IMM;
- inst.reloc.pc_rel = 0;
- inst.instruction |= shft->value;
-
- * str = p;
- return SUCCESS;
- }
- else
- {
- inst.error = (unrestrict
- ? _("shift requires register or #expression")
- : _("shift requires #expression"));
- * str = p;
- return FAIL;
}
- }
! inst.error = _("Shift expression expected");
! return FAIL;
}
/* Do those data_ops which can take a negative immediate constant
--- 2548,2634 ----
c = * p;
* p = '\0';
! shift = (struct asm_shift_name *) hash_find (arm_shift_hsh, * str);
* p = c;
!
! if (shift == NULL)
{
! inst.error = _("Shift expression expected");
! return FAIL;
! }
! assert (shift->properties->index == shift_properties[shift->properties->index].index);
!
! if (shift->properties->index == SHIFT_RRX)
! {
! * str = p;
! inst.instruction |= shift->properties->bit_field;
! return SUCCESS;
! }
! skip_whitespace (p);
! if (unrestrict && reg_required_here (& p, 8) != FAIL)
! {
! inst.instruction |= shift->properties->bit_field | SHIFT_BY_REG;
! * str = p;
! return SUCCESS;
! }
! else if (! is_immediate_prefix (* p))
! {
! inst.error = (unrestrict
! ? _("shift requires register or #expression")
! : _("shift requires #expression"));
! * str = p;
! return FAIL;
! }
!
! inst.error = NULL;
! p ++;
!
! if (my_get_expression (& inst.reloc.exp, & p))
! return FAIL;
!
! /* Validate some simple #expressions. */
! if (inst.reloc.exp.X_op == O_constant)
! {
! unsigned num = inst.reloc.exp.X_add_number;
! /* Reject operations greater than 32. */
! if (num > 32
! /* Reject a shift of 0 unless the mode allows it. */
! || (num == 0 && shift->properties->allows_0 == 0)
! /* Reject a shift of 32 unless the mode allows it. */
! || (num == 32 && shift->properties->allows_32 == 0)
! )
! {
! /* As a special case we allow ROR #0, but we issue a message
! reminding the programmer that this is actually an RRX. */
! if (num == 0 && shift->properties->index == SHIFT_ROR)
! as_tsktsk (_("ROR #0 is actually RRX"));
! else
{
! inst.error = _("Invalid immediate shift");
! return FAIL;
}
}
! /* Shifts of 32 are encoded as 0, for those shifts that
! support it. */
! if (num == 32)
! num = 0;
!
! inst.instruction |= (num << 7) | shift->properties->bit_field;
! }
! else
! {
! inst.reloc.type = BFD_RELOC_ARM_SHIFT_IMM;
! inst.reloc.pc_rel = 0;
! inst.instruction |= shift->properties->bit_field;
! }
!
! * str = p;
! return SUCCESS;
}
/* Do those data_ops which can take a negative immediate constant
*************** md_begin ()
*** 5240,5247 ****
hash_insert (arm_tops_hsh, tinsns[i].template, (PTR) (tinsns + i));
for (i = 0; i < sizeof (conds) / sizeof (struct asm_cond); i++)
hash_insert (arm_cond_hsh, conds[i].template, (PTR) (conds + i));
! for (i = 0; i < sizeof (shift) / sizeof (struct asm_shift); i++)
! hash_insert (arm_shift_hsh, shift[i].template, (PTR) (shift + i));
for (i = 0; i < sizeof (psrs) / sizeof (struct asm_psr); i++)
hash_insert (arm_psr_hsh, psrs[i].template, (PTR) (psrs + i));
--- 5270,5277 ----
hash_insert (arm_tops_hsh, tinsns[i].template, (PTR) (tinsns + i));
for (i = 0; i < sizeof (conds) / sizeof (struct asm_cond); i++)
hash_insert (arm_cond_hsh, conds[i].template, (PTR) (conds + i));
! for (i = 0; i < sizeof (shift_names) / sizeof (struct asm_shift_name); i++)
! hash_insert (arm_shift_hsh, shift_names[i].name, (PTR) (shift_names + i));
for (i = 0; i < sizeof (psrs) / sizeof (struct asm_psr); i++)
hash_insert (arm_psr_hsh, psrs[i].template, (PTR) (psrs + i));
Index: gas/testsuite/gas/arm/inst.s
===================================================================
RCS file: /cvs/src//src/gas/testsuite/gas/arm/inst.s,v
retrieving revision 1.4
diff -p -r1.4 inst.s
*** inst.s 2000/01/31 22:14:50 1.4
--- inst.s 2000/08/16 18:57:55
*************** bar:
*** 187,189 ****
--- 187,223 ----
blpl hohum
b _wibble
ble testerfunc
+
+ mov r1, r2, lsl #2
+ mov r1, r2, lsl #0
+ mov r1, r2, lsl #31
+ mov r1, r2, lsl r3
+ mov r1, r2, lsr #2
+ mov r1, r2, lsr #31
+ mov r1, r2, lsr #32
+ mov r1, r2, lsr r3
+ mov r1, r2, asr #2
+ mov r1, r2, asr #31
+ mov r1, r2, asr #32
+ mov r1, r2, asr r3
+ mov r1, r2, ror #2
+ mov r1, r2, ror #31
+ mov r1, r2, ror r3
+ mov r1, r2, rrx
+ mov r1, r2, LSL #2
+ mov r1, r2, LSL #0
+ mov r1, r2, LSL #31
+ mov r1, r2, LSL r3
+ mov r1, r2, LSR #2
+ mov r1, r2, LSR #31
+ mov r1, r2, LSR #32
+ mov r1, r2, LSR r3
+ mov r1, r2, ASR #2
+ mov r1, r2, ASR #31
+ mov r1, r2, ASR #32
+ mov r1, r2, ASR r3
+ mov r1, r2, ROR #2
+ mov r1, r2, ROR #31
+ mov r1, r2, ROR r3
+ mov r1, r2, RRX
+
\ No newline at end of file
Index: gas/testsuite/gas/arm/inst.d
===================================================================
RCS file: /cvs/src//src/gas/testsuite/gas/arm/inst.d,v
retrieving revision 1.5
diff -p -r1.5 inst.d
*** inst.d 2000/07/18 22:07:53 1.5
--- inst.d 2000/08/16 18:57:55
*************** Disassembly of section .text:
*** 167,169 ****
--- 167,201 ----
[ ]*268:.*_wibble.*
0000026c <[^>]*> dafffffe ? ble 0000026c <[^>]*>
[ ]*26c:.*testerfunc.*
+ 00000270 <[^>]*> e1a01102 ? mov r1, r2, lsl #2
+ 00000274 <[^>]*> e1a01002 ? mov r1, r2
+ 00000278 <[^>]*> e1a01f82 ? mov r1, r2, lsl #31
+ 0000027c <[^>]*> e1a01312 ? mov r1, r2, lsl r3
+ 00000280 <[^>]*> e1a01122 ? mov r1, r2, lsr #2
+ 00000284 <[^>]*> e1a01fa2 ? mov r1, r2, lsr #31
+ 00000288 <[^>]*> e1a01022 ? mov r1, r2, lsr #32
+ 0000028c <[^>]*> e1a01332 ? mov r1, r2, lsr r3
+ 00000290 <[^>]*> e1a01142 ? mov r1, r2, asr #2
+ 00000294 <[^>]*> e1a01fc2 ? mov r1, r2, asr #31
+ 00000298 <[^>]*> e1a01042 ? mov r1, r2, asr #32
+ 0000029c <[^>]*> e1a01352 ? mov r1, r2, asr r3
+ 000002a0 <[^>]*> e1a01162 ? mov r1, r2, ror #2
+ 000002a4 <[^>]*> e1a01fe2 ? mov r1, r2, ror #31
+ 000002a8 <[^>]*> e1a01372 ? mov r1, r2, ror r3
+ 000002ac <[^>]*> e1a01062 ? mov r1, r2, rrx
+ 000002b0 <[^>]*> e1a01102 ? mov r1, r2, lsl #2
+ 000002b4 <[^>]*> e1a01002 ? mov r1, r2
+ 000002b8 <[^>]*> e1a01f82 ? mov r1, r2, lsl #31
+ 000002bc <[^>]*> e1a01312 ? mov r1, r2, lsl r3
+ 000002c0 <[^>]*> e1a01122 ? mov r1, r2, lsr #2
+ 000002c4 <[^>]*> e1a01fa2 ? mov r1, r2, lsr #31
+ 000002c8 <[^>]*> e1a01022 ? mov r1, r2, lsr #32
+ 000002cc <[^>]*> e1a01332 ? mov r1, r2, lsr r3
+ 000002d0 <[^>]*> e1a01142 ? mov r1, r2, asr #2
+ 000002d4 <[^>]*> e1a01fc2 ? mov r1, r2, asr #31
+ 000002d8 <[^>]*> e1a01042 ? mov r1, r2, asr #32
+ 000002dc <[^>]*> e1a01352 ? mov r1, r2, asr r3
+ 000002e0 <[^>]*> e1a01162 ? mov r1, r2, ror #2
+ 000002e4 <[^>]*> e1a01fe2 ? mov r1, r2, ror #31
+ 000002e8 <[^>]*> e1a01372 ? mov r1, r2, ror r3
+ 000002ec <[^>]*> e1a01062 ? mov r1, r2, rrx