This is the mail archive of the binutils@sourceware.org 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: [PATCH, ARM] Fix parsing of int/float immediates for Neon VMOV instructions


Paul Brook wrote:
To use floating-point immediates, you must now explicitly use a decimal
point or exponent, i.e.

vmov.f32 dX, #10.0

not

vmov.f32 dX, #10

This would be ok if we rejected the latter. However IIUC after your patch we silently encode the latter as vmov.i32 dX, #10. This is equivalent to vmov.f32 dX, #<some denormal or unrepresentable FP value> which is almost certainly not what the user intended.

This version of the patch will reject the latter case instead, which I agree seems far more sensible.


Re-tested (with a minor testsuite tweak) with arm-none-eabi cross.

OK now?

Cheers,

Julian

ChangeLog

    gas/
    * config/tc-arm.c (arm_it): Add immisfloat field.
    (parse_qfloat_immediate): Disallow integer syntax for floating-point
    immediates. Fix hex immediates, handle 0.0 and -0.0 specially.
    (parse_neon_mov): Set immisfloat bit for operand if it parsed as a
    float.
    (neon_cmode_for_move_imm): Reject non-float immediates for float
    operands.
    (neon_move_immediate): Pass immisfloat bit to
    neon_cmode_for_move_imm.

    gas/testsuite/
    * gas/arm/neon-const.s: Use FP syntax for 0/-0.
    * gas/arm/vfp-neon-syntax-inc.s: Likewise, for 1.
    * gas/arm/neon-cov.s: Use float syntax for FP immediate.
Index: gas/config/tc-arm.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-arm.c,v
retrieving revision 1.311
diff -c -p -r1.311 tc-arm.c
*** gas/config/tc-arm.c	25 Feb 2007 19:29:25 -0000	1.311
--- gas/config/tc-arm.c	2 Mar 2007 18:17:15 -0000
*************** struct arm_it
*** 333,338 ****
--- 333,339 ----
      unsigned immisreg	: 1;  /* .imm field is a second register.  */
      unsigned isscalar   : 1;  /* Operand is a (Neon) scalar.  */
      unsigned immisalign : 1;  /* Immediate is an alignment specifier.  */
+     unsigned immisfloat : 1;  /* Immediate was parsed as a float.  */
      /* Note: we abuse "regisimm" to mean "is Neon register" in VMOV
         instructions. This allows us to disambiguate ARM <-> vector insns.  */
      unsigned regisimm   : 1;  /* 64-bit immediate, reg forms high 32 bits.  */
*************** is_quarter_float (unsigned imm)
*** 4177,4194 ****
  
  /* Parse an 8-bit "quarter-precision" floating point number of the form:
     0baBbbbbbc defgh000 00000000 00000000.
!    The minus-zero case needs special handling, since it can't be encoded in the
!    "quarter-precision" float format, but can nonetheless be loaded as an integer
!    constant.  */
  
  static unsigned
  parse_qfloat_immediate (char **ccp, int *immed)
  {
    char *str = *ccp;
    LITTLENUM_TYPE words[MAX_LITTLENUMS];
    
    skip_past_char (&str, '#');
    
    if ((str = atof_ieee (str, 's', words)) != NULL)
      {
        unsigned fpword = 0;
--- 4178,4220 ----
  
  /* Parse an 8-bit "quarter-precision" floating point number of the form:
     0baBbbbbbc defgh000 00000000 00000000.
!    The zero and minus-zero cases need special handling, since they can't be
!    encoded in the "quarter-precision" float format, but can nonetheless be
!    loaded as integer constants.  */
  
  static unsigned
  parse_qfloat_immediate (char **ccp, int *immed)
  {
    char *str = *ccp;
+   char *fpnum;
    LITTLENUM_TYPE words[MAX_LITTLENUMS];
+   int found_fpchar = 0;
    
    skip_past_char (&str, '#');
    
+   /* We must not accidentally parse an integer as a floating-point number. Make
+      sure that the value we parse is not an integer by checking for special
+      characters '.' or 'e'.
+      FIXME: This is a horrible hack, but doing better is tricky because type
+      information isn't in a very usable state at parse time.  */
+   fpnum = str;
+   skip_whitespace (fpnum);
+ 
+   if (strncmp (fpnum, "0x", 2) == 0)
+     return FAIL;
+   else
+     {
+       for (; *fpnum != '\0' && *fpnum != ' ' && *fpnum != '\n'; fpnum++)
+         if (*fpnum == '.' || *fpnum == 'e' || *fpnum == 'E')
+           {
+             found_fpchar = 1;
+             break;
+           }
+ 
+       if (!found_fpchar)
+         return FAIL;
+     }
+   
    if ((str = atof_ieee (str, 's', words)) != NULL)
      {
        unsigned fpword = 0;
*************** parse_qfloat_immediate (char **ccp, int 
*** 4201,4207 ****
            fpword |= words[i];
          }
        
!       if (is_quarter_float (fpword) || fpword == 0x80000000)
          *immed = fpword;
        else
          return FAIL;
--- 4227,4233 ----
            fpword |= words[i];
          }
        
!       if (is_quarter_float (fpword) || (fpword & 0x7fffffff) == 0)
          *immed = fpword;
        else
          return FAIL;
*************** parse_neon_mov (char **str, int *which_o
*** 5201,5207 ****
               Case 3: VMOV<c><q>.<dt> <Dd>, #<float-imm>
               Case 10: VMOV.F32 <Sd>, #<imm>
               Case 11: VMOV.F64 <Dd>, #<imm>  */
!         ;
        else if (parse_big_immediate (&ptr, i) == SUCCESS)
            /* Case 2: VMOV<c><q>.<dt> <Qd>, #<imm>
               Case 3: VMOV<c><q>.<dt> <Dd>, #<imm>  */
--- 5227,5233 ----
               Case 3: VMOV<c><q>.<dt> <Dd>, #<float-imm>
               Case 10: VMOV.F32 <Sd>, #<imm>
               Case 11: VMOV.F64 <Dd>, #<imm>  */
!         inst.operands[i].immisfloat = 1;
        else if (parse_big_immediate (&ptr, i) == SUCCESS)
            /* Case 2: VMOV<c><q>.<dt> <Qd>, #<imm>
               Case 3: VMOV<c><q>.<dt> <Dd>, #<imm>  */
*************** neon_qfloat_bits (unsigned imm)
*** 11550,11558 ****
     try smaller element sizes.  */
  
  static int
! neon_cmode_for_move_imm (unsigned immlo, unsigned immhi, unsigned *immbits,
!                          int *op, int size, enum neon_el_type type)
! {
    if (type == NT_float && is_quarter_float (immlo) && immhi == 0)
      {
        if (size != 32 || *op == 1)
--- 11576,11590 ----
     try smaller element sizes.  */
  
  static int
! neon_cmode_for_move_imm (unsigned immlo, unsigned immhi, int float_p,
! 			 unsigned *immbits, int *op, int size,
! 			 enum neon_el_type type)
! {
!   /* Only permit float immediates (including 0.0/-0.0) if the operand type is
!      float.  */
!   if (type == NT_float && !float_p)
!     return FAIL;
! 
    if (type == NT_float && is_quarter_float (immlo) && immhi == 0)
      {
        if (size != 32 || *op == 1)
*************** neon_move_immediate (void)
*** 12548,12554 ****
    struct neon_type_el et = neon_check_type (2, rs,
      N_I8 | N_I16 | N_I32 | N_I64 | N_F32 | N_KEY, N_EQK);
    unsigned immlo, immhi = 0, immbits;
!   int op, cmode;
  
    constraint (et.type == NT_invtype,
                _("operand size must be specified for immediate VMOV"));
--- 12580,12586 ----
    struct neon_type_el et = neon_check_type (2, rs,
      N_I8 | N_I16 | N_I32 | N_I64 | N_F32 | N_KEY, N_EQK);
    unsigned immlo, immhi = 0, immbits;
!   int op, cmode, float_p;
  
    constraint (et.type == NT_invtype,
                _("operand size must be specified for immediate VMOV"));
*************** neon_move_immediate (void)
*** 12563,12569 ****
    constraint (et.size < 32 && (immlo & ~((1 << et.size) - 1)) != 0,
                _("immediate has bits set outside the operand size"));
  
!   if ((cmode = neon_cmode_for_move_imm (immlo, immhi, &immbits, &op,
                                          et.size, et.type)) == FAIL)
      {
        /* Invert relevant bits only.  */
--- 12595,12603 ----
    constraint (et.size < 32 && (immlo & ~((1 << et.size) - 1)) != 0,
                _("immediate has bits set outside the operand size"));
  
!   float_p = inst.operands[1].immisfloat;
! 
!   if ((cmode = neon_cmode_for_move_imm (immlo, immhi, float_p, &immbits, &op,
                                          et.size, et.type)) == FAIL)
      {
        /* Invert relevant bits only.  */
*************** neon_move_immediate (void)
*** 12572,12579 ****
           with one or the other; those cases are caught by
           neon_cmode_for_move_imm.  */
        op = !op;
!       if ((cmode = neon_cmode_for_move_imm (immlo, immhi, &immbits, &op,
!                                             et.size, et.type)) == FAIL)
          {
            first_error (_("immediate out of range"));
            return;
--- 12606,12613 ----
           with one or the other; those cases are caught by
           neon_cmode_for_move_imm.  */
        op = !op;
!       if ((cmode = neon_cmode_for_move_imm (immlo, immhi, float_p, &immbits,
! 					    &op, et.size, et.type)) == FAIL)
          {
            first_error (_("immediate out of range"));
            return;
Index: gas/testsuite/gas/arm/neon-const.s
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/arm/neon-const.s,v
retrieving revision 1.1
diff -c -p -r1.1 neon-const.s
*** gas/testsuite/gas/arm/neon-const.s	26 Apr 2006 16:02:40 -0000	1.1
--- gas/testsuite/gas/arm/neon-const.s	2 Mar 2007 18:17:15 -0000
***************
*** 4,10 ****
  	.text
  	.syntax unified
  
!         vmov.f32 q0, 0
  
  	vmov.f32 q0, 2.0
          vmov.f32 q0, 4.0
--- 4,10 ----
  	.text
  	.syntax unified
  
!         vmov.f32 q0, 0.0
  
  	vmov.f32 q0, 2.0
          vmov.f32 q0, 4.0
***************
*** 150,156 ****
          vmov.f32 q0, 0.96875
          vmov.f32 q0, 1.9375
  
!         vmov.f32 q0, -0
  
  	vmov.f32 q0, -2.0
          vmov.f32 q0, -4.0
--- 150,156 ----
          vmov.f32 q0, 0.96875
          vmov.f32 q0, 1.9375
  
!         vmov.f32 q0, -0.0
  
  	vmov.f32 q0, -2.0
          vmov.f32 q0, -4.0
Index: gas/testsuite/gas/arm/neon-cov.s
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/arm/neon-cov.s,v
retrieving revision 1.4
diff -c -p -r1.4 neon-cov.s
*** gas/testsuite/gas/arm/neon-cov.s	8 Oct 2006 18:44:07 -0000	1.4
--- gas/testsuite/gas/arm/neon-cov.s	2 Mar 2007 18:17:15 -0000
***************
*** 456,462 ****
  	mov_imm vmvn 0x0077ffff .i32
  	mov_imm vmov 0x77 .i8
  	mov_imm vmov 0xff0000ff000000ff .i64
! 	mov_imm vmov 0x40880000 .f32
  
  	mov_imm vmov 0xa5a5 .i16
  	mov_imm vmvn 0xa5a5 .i16
--- 456,462 ----
  	mov_imm vmvn 0x0077ffff .i32
  	mov_imm vmov 0x77 .i8
  	mov_imm vmov 0xff0000ff000000ff .i64
! 	mov_imm vmov 4.25 .f32
  
  	mov_imm vmov 0xa5a5 .i16
  	mov_imm vmvn 0xa5a5 .i16
Index: gas/testsuite/gas/arm/vfp-neon-syntax-inc.s
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/arm/vfp-neon-syntax-inc.s,v
retrieving revision 1.2
diff -c -p -r1.2 vfp-neon-syntax-inc.s
*** gas/testsuite/gas/arm/vfp-neon-syntax-inc.s	7 Jun 2006 14:31:51 -0000	1.2
--- gas/testsuite/gas/arm/vfp-neon-syntax-inc.s	2 Mar 2007 18:17:15 -0000
*************** func:
*** 9,15 ****
          vmov\cond\f32 s0,s1
          vmov\cond\f64 d0,d1
          vmov\cond\f32 s0,#0.25
!         vmov\cond\f64 d0,#1
  	itblock 4 \cond
          vmov\cond r0,s1
          vmov\cond s0,r1
--- 9,15 ----
          vmov\cond\f32 s0,s1
          vmov\cond\f64 d0,d1
          vmov\cond\f32 s0,#0.25
!         vmov\cond\f64 d0,#1.0
  	itblock 4 \cond
          vmov\cond r0,s1
          vmov\cond s0,r1

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