This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH][ARM][gas] Make gas accept vcmp[e].f{32,64} <reg>, #0.0
- From: Kyrill Tkachov <kyrylo dot tkachov at arm dot com>
- To: Will Newton <will dot newton at linaro dot org>
- Cc: "binutils at sourceware dot org" <binutils at sourceware dot org>, Richard Earnshaw <Richard dot Earnshaw at arm dot com>, "nickc at redhat dot com" <nickc at redhat dot com>
- Date: Thu, 07 Aug 2014 09:45:25 +0100
- Subject: Re: [PATCH][ARM][gas] Make gas accept vcmp[e].f{32,64} <reg>, #0.0
- Authentication-results: sourceware.org; auth=none
- References: <53DF539A dot 2010203 at arm dot com> <CANu=DmgbBDt0Ym7HMgnUabGwH88jGB+4L7Mk+Yqrc2KgHfVWtA at mail dot gmail dot com> <53E0D7E5 dot 1040107 at arm dot com> <CANu=Dmi+arpsw65UXVd=NLW_dYmL=rwOw-Wdi+p6mXERd+MQSA at mail dot gmail dot com>
On 05/08/14 15:14, Will Newton wrote:
On 5 August 2014 14:11, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
On 04/08/14 11:08, Will Newton wrote:
On 4 August 2014 10:34, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
Hi Kyrill,
The ARM ARM says that the VFP compare with zero instructions (vcmp,
vcmpe)
accept the zero in the form of a #0.0 immediate. However, gas currently
only
accepts #0. This patch fixes that and some simple tests are added.
The new parse_ifimm_zerofunction is rather simple and is modelled on the
parse_qfloat_immediate below it and didn't blow up in my testing.
make check on gas passes and a gcc testsuite run doesn't show any
problems.
Ok for trunk?
Thanks,
Kyrill
[gas/]
2014-06-24 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
* config/tc-arm.c (parse_ifimm_zero): New function.
(enum operand_parse_code): Add OP_RSVD_FI0 value.
(parse_operands): Handle OP_RSVD_FI0.
(asm_opcode_insns): Use RSVD_FI0 for second operand of vcmp, vcmpe.
[gas/testsuite/]
2014-06-24 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
* gas/arm/ual-vcmp.s: New file.
* gas/arm/ual-vcmp.d: Likewise.
Would it be possible to use my_get_expression or a similar parser for
this?
Hi Will,
Thanks for the suggestion, but I'm struggling to figure out what
my_get_expression is supposed to be doing. Is it supposed to be taking a
string and parsing it into an expressionS?
I'm not very well versed in gas internal data types...
Yes, that's pretty much it. I think an integer will be type O_constant
with the value in X_add_number and a floating point value O_big with a
value in generic_floating_point_number. The expr.c code is not pretty
but it should be possible to make it do something approaching what's
reguired...
Hi Will,
After a bit of investigation, it seems that atof_generic is what I need.
It can handle any form of floating point zero, as well as plain old "0".
From what I understand from looking around gas the global variable generic_floating_point_number is supposed to be used to store the various parts of a floating point representation and the way it's checked for 0.0 in config/atof-ieee.c is by comparing the .low and .leader components.
I've also added a *-bad.s testcase to make sure we reject non-zero immediates.
make check on gas passes.
How does this look?
Thanks,
Kyrill
[gas/]
2014-08-07 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
* config/tc-arm.c (parse_ifimm_zero): New function.
(enum operand_parse_code): Add OP_RSVD_FI0 value.
(parse_operands): Handle OP_RSVD_FI0.
(asm_opcode_insns): Use RSVD_FI0 for second operand of vcmp, vcmpe.
[gas/testsuite/]
2014-08-07 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
* gas/arm/ual-vcmp.s: New file.
* gas/arm/ual-vcmp.d: Likewise.
* gas/arm/vcmp-zero-bad.s: Likewise.
* gas/arm/vcmp-zero-bad.d: Likewise.
* gas/arm/vcmp-zero-bad.l: Likewise.
commit 7b1b91efafec2cb5c2946c9141e6432ad1a5d273
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date: Thu Jul 24 15:24:55 2014 +0100
[ARM][gas] Accept vcmp <reg>, #0.0
diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c
index 75fb233..5356a07 100644
--- a/gas/config/tc-arm.c
+++ b/gas/config/tc-arm.c
@@ -249,6 +249,8 @@ static arm_feature_set selected_cpu = ARM_ARCH_NONE;
/* Must be long enough to hold any of the names in arm_cpus. */
static char selected_cpu_name[16];
+extern FLONUM_TYPE generic_floating_point_number;
+
/* Return if no cpu was selected on command-line. */
static bfd_boolean
no_cpu_selected (void)
@@ -4944,6 +4946,31 @@ is_quarter_float (unsigned imm)
return (imm & 0x7ffff) == 0 && ((imm & 0x7e000000) ^ bs) == 0;
}
+
+/* Detect the presence of a floating point or integer zero constant,
+ i.e. #0.0 or #0. */
+
+static bfd_boolean
+parse_ifimm_zero (char **in)
+{
+ int error_code;
+
+ if (!is_immediate_prefix (**in))
+ return FALSE;
+
+ ++*in;
+ error_code = atof_generic (in, ".", EXP_CHARS,
+ &generic_floating_point_number);
+
+ if (!error_code
+ && generic_floating_point_number.sign == '+'
+ && (generic_floating_point_number.low
+ > generic_floating_point_number.leader))
+ return TRUE;
+
+ return FALSE;
+}
+
/* 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
@@ -6417,6 +6444,7 @@ enum operand_parse_code
OP_RNDQ_I0, /* Neon D or Q reg, or immediate zero. */
OP_RVSD_I0, /* VFP S or D reg, or immediate zero. */
+ OP_RSVD_FI0, /* VFP S or D reg, or floating point immediate zero. */
OP_RR_RNSC, /* ARM reg or Neon scalar. */
OP_RNSDQ_RNSC, /* Vector S, D or Q reg, or Neon scalar. */
OP_RNDQ_RNSC, /* Neon D or Q reg, or Neon scalar. */
@@ -6700,6 +6728,22 @@ parse_operands (char *str, const unsigned int *pattern, bfd_boolean thumb)
po_reg_or_goto (REG_TYPE_VFSD, try_imm0);
break;
+ case OP_RSVD_FI0:
+ {
+ po_reg_or_goto (REG_TYPE_VFSD, try_ifimm0);
+ break;
+ try_ifimm0:
+ if (parse_ifimm_zero (&str))
+ inst.operands[i].imm = 0;
+ else
+ {
+ inst.error
+ = _("only floating point zero is allowed as immediate value");
+ goto failure;
+ }
+ }
+ break;
+
case OP_RR_RNSC:
{
po_scalar_or_goto (8, try_rr);
@@ -19552,8 +19596,8 @@ static const struct asm_opcode insns[] =
nCE(vnmul, _vnmul, 3, (RVSD, RVSD, RVSD), vfp_nsyn_nmul),
nCE(vnmla, _vnmla, 3, (RVSD, RVSD, RVSD), vfp_nsyn_nmul),
nCE(vnmls, _vnmls, 3, (RVSD, RVSD, RVSD), vfp_nsyn_nmul),
- nCE(vcmp, _vcmp, 2, (RVSD, RVSD_I0), vfp_nsyn_cmp),
- nCE(vcmpe, _vcmpe, 2, (RVSD, RVSD_I0), vfp_nsyn_cmp),
+ nCE(vcmp, _vcmp, 2, (RVSD, RSVD_FI0), vfp_nsyn_cmp),
+ nCE(vcmpe, _vcmpe, 2, (RVSD, RSVD_FI0), vfp_nsyn_cmp),
NCE(vpush, 0, 1, (VRSDLST), vfp_nsyn_push),
NCE(vpop, 0, 1, (VRSDLST), vfp_nsyn_pop),
NCE(vcvtz, 0, 2, (RVSD, RVSD), vfp_nsyn_cvtz),
diff --git a/gas/testsuite/gas/arm/ual-vcmp.d b/gas/testsuite/gas/arm/ual-vcmp.d
new file mode 100644
index 0000000..2e9fb3d
--- /dev/null
+++ b/gas/testsuite/gas/arm/ual-vcmp.d
@@ -0,0 +1,24 @@
+#objdump: -dr --prefix-addresses --show-raw-insn
+#name: UAL vcmp with 0
+#as: -mfpu=vfpv3
+
+.*: +file format .*arm.*
+
+
+Disassembly of section .text:
+0+000 <[^>]*> eeb50a40 vcmp.f32 s0, #0.0
+0+004 <[^>]*> eef50a40 vcmp.f32 s1, #0.0
+0+008 <[^>]*> eef51ac0 vcmpe.f32 s3, #0.0
+0+00c <[^>]*> eeb52ac0 vcmpe.f32 s4, #0.0
+0+010 <[^>]*> eef52a40 vcmp.f32 s5, #0.0
+0+014 <[^>]*> eeb53a40 vcmp.f32 s6, #0.0
+0+018 <[^>]*> eef53ac0 vcmpe.f32 s7, #0.0
+0+01c <[^>]*> eeb54ac0 vcmpe.f32 s8, #0.0
+0+020 <[^>]*> eeb50b40 vcmp.f64 d0, #0.0
+0+024 <[^>]*> eeb51b40 vcmp.f64 d1, #0.0
+0+028 <[^>]*> eeb52bc0 vcmpe.f64 d2, #0.0
+0+02c <[^>]*> eeb53bc0 vcmpe.f64 d3, #0.0
+0+030 <[^>]*> eeb54b40 vcmp.f64 d4, #0.0
+0+034 <[^>]*> eeb55b40 vcmp.f64 d5, #0.0
+0+038 <[^>]*> eeb56bc0 vcmpe.f64 d6, #0.0
+0+03c <[^>]*> eeb57bc0 vcmpe.f64 d7, #0.0
\ No newline at end of file
diff --git a/gas/testsuite/gas/arm/ual-vcmp.s b/gas/testsuite/gas/arm/ual-vcmp.s
new file mode 100644
index 0000000..79efa4e
--- /dev/null
+++ b/gas/testsuite/gas/arm/ual-vcmp.s
@@ -0,0 +1,21 @@
+.text
+.arm
+.syntax unified
+
+vcmp.f32 s0, #0.0
+vcmp.f32 s1, #0
+vcmpe.f32 s3, #0.0
+vcmpe.f32 s4, #0
+vcmp.f32 s5, #0.0e2
+vcmp.f32 s6, #0e-3
+vcmpe.f32 s7, #0.0000
+vcmpe.f32 s8, #.0
+
+vcmp.f64 d0, #0.0
+vcmp.f64 d1, #0
+vcmpe.f64 d2, #0.0
+vcmpe.f64 d3, #0
+vcmp.f64 d4, #0.0e5
+vcmp.f64 d5, #0e-2
+vcmpe.f64 d6, #0.00
+vcmpe.f64 d7, #.0
diff --git a/gas/testsuite/gas/arm/vcmp-zero-bad.d b/gas/testsuite/gas/arm/vcmp-zero-bad.d
new file mode 100644
index 0000000..806cf00
--- /dev/null
+++ b/gas/testsuite/gas/arm/vcmp-zero-bad.d
@@ -0,0 +1,4 @@
+# name: UAL vcmp with #0.0 bad instructions.
+# as: -mfpu=vfpv3
+# source: vcmp-zero-bad.s
+# error-output: vcmp-zero-bad.l
diff --git a/gas/testsuite/gas/arm/vcmp-zero-bad.l b/gas/testsuite/gas/arm/vcmp-zero-bad.l
new file mode 100644
index 0000000..975d02b
--- /dev/null
+++ b/gas/testsuite/gas/arm/vcmp-zero-bad.l
@@ -0,0 +1,8 @@
+[^:]*: Assembler messages:
+[^:]*:5: Error: only floating point zero is allowed as immediate value -- `vcmp.f32 s0,#0.01'
+[^:]*:6: Error: only floating point zero is allowed as immediate value -- `vcmp.f32 s1,#2'
+[^:]*:7: Error: only floating point zero is allowed as immediate value -- `vcmpe.f32 s3,5'
+[^:]*:8: Error: only floating point zero is allowed as immediate value -- `vcmpe.f32 s4,#-0.0'
+[^:]*:10: Error: only floating point zero is allowed as immediate value -- `vcmp.f64 d0,#-1'
+[^:]*:11: Error: garbage following instruction -- `vcmpe.f64 d3,#0x35'
+[^:]*:12: Error: only floating point zero is allowed as immediate value -- `vcmpe.f64 d4,0xf'
diff --git a/gas/testsuite/gas/arm/vcmp-zero-bad.s b/gas/testsuite/gas/arm/vcmp-zero-bad.s
new file mode 100644
index 0000000..3be79cb
--- /dev/null
+++ b/gas/testsuite/gas/arm/vcmp-zero-bad.s
@@ -0,0 +1,12 @@
+.text
+.arm
+.syntax unified
+
+vcmp.f32 s0, #0.01
+vcmp.f32 s1, #2
+vcmpe.f32 s3, 5
+vcmpe.f32 s4, #-0.0
+
+vcmp.f64 d0, #-1
+vcmpe.f64 d3, #0x35
+vcmpe.f64 d4, 0xf