[PATCH] Allow encoded 8-bit floating point constants in ARM vmov instructions

David Peixotto dpeixott at codeaurora.org
Fri Dec 20 12:12:44 PST 2013 

On 12/19/2013 7:09 AM, Renato Golin wrote:
> So, FCONSTD does accept encoded floats, but that's a deprecated
> instruction and, as Jim said, we should not support it unless it's
> either impossible not to, or it's just an alias to a UAL instruction.
> The current ARM assembler (and GNU, too) doesn't support encoded floats
> as immediate values.
>
> FCONSTD:
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0204g/CJAEFGHE.html
>
> VMOV.F:
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0204j/CJAEFGHE.html
>
> Let me gather the issues...
>
> 1. VMOV.f64 had support for encoded literals, seems accidental,
> shouldn't have been there. How do we "fix" this?

We can easily fix this by not attempting to parse the encoded literal 
for vmov.f instructions.

> 2. FCONSTD is pre-UAL, deprecated, and it's not a direct alias to VMOV.F
> (as it supports encoded imm). In theory, we'd say *no* to that.

Yes, it is not a direct alias because of the different operand syntax 
(8-bit encoded float vs. float literal).

> 3. Adding support for encoded FP in VMOV could create a new range of
> LLVM extensions, and we don't want that.

Agreed, I do not see any reason to add that extension.

> 4. Support is already there, we could say we only support encoded FPs if
> using pre-UAL syntax.

Right, all the hooks are in place.

> My view is that fixing VMOV's encoded FP wouldn't bloat the code, but
> would start a precedent (LLVM extensions). Adding FCONSTD as an alias to
> VMOV should be ok, if it did support encoded imms, but we should be able
> to explicitly allow encoded values *only* if the instruction is FCONSTD,
> and not VMOV.F. That would not add any extension on our part.

To make things concrete, I have attached a patch that fixes issue #1 and 
adds the pre-UAL aliases. It only accepts encoded floats for the fconst 
aliases.

> IFF this is possible, AND doesn't bloat the code, I don't mind it being
> added. Jim might have a different opinion...

Please take a look at the patch and let me know your opinion.
-------------- next part --------------
>From b0049ec7e0e0c7c7d9f8c68ec5f068aecc1fcdde Mon Sep 17 00:00:00 2001
From: David Peixotto <dpeixott at codeaurora.org>
Date: Fri, 13 Dec 2013 13:53:03 -0800
Subject: [PATCH] Add ARM fconsts/fconstd aliases for vmov.f32/vmov.f64

This commit adds the pre-UAL aliases of fconsts and fconstd for
vmov.f32 and vmov.f64. They use an InstAlias rather than a
MnemonicAlias to properly support the predicate operand.

We need to support encoded 8-bit constants in order to implement the
pre-UAL fconsts/fconstd aliases for vmov.f32/vmov.f64, so this
commit also fixes parsing of encoded floating point constants used
in vmov.f32/vmov.f64 instructions. Now we can support assembly code
like this:

  fconsts s0, #0x70

which is equivalent to vmov.f32 s0, #1.0.

Most of the code was already in place to support this feature.
Previously the code was trying to accept encoded 8-bit float
constants for the vmov.f32/vmov.f64 instructions.  It looks like the
support for parsing encoded floats was lost in a refactoring in
commit r148556 and we did not have any tests in place to catch it.

The change in this commit is to keep the parsed value as a 32-bit
float instead of a 64-bit double because that is what the isFPImm()
function expects to find. There is no loss of precision by using a
32-bit float here because we are still limited to an 8-bit encoded
value in the end.

Additionally, we explicitly reject encoded 8-bit floats for
vmovf.32/64. This is the same as the current behavior, but we now do
it explicitly rather than accidently.
---
 lib/Target/ARM/ARMInstrVFP.td             |    6 ++++
 lib/Target/ARM/AsmParser/ARMAsmParser.cpp |   19 ++++++++-----
 test/MC/ARM/fconst.s                      |   22 +++++++++++++++
 test/MC/ARM/fp-const-errors.s             |   22 +++++++++++++++
 test/MC/ARM/simple-fp-encoding.s          |   43 +++++++++++++++++++++++++++++
 5 files changed, 105 insertions(+), 7 deletions(-)
 create mode 100644 test/MC/ARM/fconst.s
 create mode 100644 test/MC/ARM/fp-const-errors.s

diff --git a/lib/Target/ARM/ARMInstrVFP.td b/lib/Target/ARM/ARMInstrVFP.td
index a8cdc5c..5f9ac8b 100644
--- a/lib/Target/ARM/ARMInstrVFP.td
+++ b/lib/Target/ARM/ARMInstrVFP.td
@@ -1735,3 +1735,9 @@ def : VFP2InstAlias<"vmov${p}.f64 $Dn, $Rt, $Rt2",
 // VMOVD does.
 def : VFP2InstAlias<"vmov${p} $Sd, $Sm",
                     (VMOVS SPR:$Sd, SPR:$Sm, pred:$p)>;
+
+// FCONSTD/FCONSTS alias for vmov.f64/vmov.f32
+def : VFP3InstAlias<"fconstd${p} $Dd, $val",
+                    (FCONSTD DPR:$Dd, vfp_f64imm:$val, pred:$p)>;
+def : VFP3InstAlias<"fconsts${p} $Sd, $val",
+                    (FCONSTS SPR:$Sd, vfp_f32imm:$val, pred:$p)>;
diff --git a/lib/Target/ARM/AsmParser/ARMAsmParser.cpp b/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
index d3b6c78..af28de3 100644
--- a/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
+++ b/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
@@ -4569,8 +4569,12 @@ parseFPImm(SmallVectorImpl<MCParsedAsmOperand*> &Operands) {
   // for these:
   // vmov.i{8|16|32|64} <dreg|qreg>, #imm
   ARMOperand *TyOp = static_cast<ARMOperand*>(Operands[2]);
-  if (!TyOp->isToken() || (TyOp->getToken() != ".f32" &&
-                           TyOp->getToken() != ".f64"))
+  bool isVmovf = TyOp->isToken() && (TyOp->getToken() == ".f32" ||
+                                     TyOp->getToken() == ".f64");
+  ARMOperand *Mnemonic = static_cast<ARMOperand*>(Operands[0]);
+  bool isFconst = Mnemonic->isToken() && (Mnemonic->getToken() == "fconstd" ||
+                                          Mnemonic->getToken() == "fconsts");
+  if (!(isVmovf || isFconst))
     return MatchOperand_NoMatch;
 
   Parser.Lex(); // Eat '#' or '$'.
@@ -4583,7 +4587,7 @@ parseFPImm(SmallVectorImpl<MCParsedAsmOperand*> &Operands) {
   }
   const AsmToken &Tok = Parser.getTok();
   SMLoc Loc = Tok.getLoc();
-  if (Tok.is(AsmToken::Real)) {
+  if (Tok.is(AsmToken::Real) && isVmovf) {
     APFloat RealVal(APFloat::IEEEsingle, Tok.getString());
     uint64_t IntVal = RealVal.bitcastToAPInt().getZExtValue();
     // If we had a '-' in front, toggle the sign bit.
@@ -4596,15 +4600,16 @@ parseFPImm(SmallVectorImpl<MCParsedAsmOperand*> &Operands) {
   }
   // Also handle plain integers. Instructions which allow floating point
   // immediates also allow a raw encoded 8-bit value.
-  if (Tok.is(AsmToken::Integer)) {
+  if (Tok.is(AsmToken::Integer) && isFconst) {
     int64_t Val = Tok.getIntVal();
     Parser.Lex(); // Eat the token.
     if (Val > 255 || Val < 0) {
       Error(Loc, "encoded floating point value out of range");
       return MatchOperand_ParseFail;
     }
-    double RealVal = ARM_AM::getFPImmFloat(Val);
-    Val = APFloat(APFloat::IEEEdouble, RealVal).bitcastToAPInt().getZExtValue();
+    float RealVal = ARM_AM::getFPImmFloat(Val);
+    Val = APFloat(RealVal).bitcastToAPInt().getZExtValue();
+
     Operands.push_back(ARMOperand::CreateImm(
         MCConstantExpr::Create(Val, getContext()), S,
         Parser.getTok().getLoc()));
@@ -4859,7 +4864,7 @@ StringRef ARMAsmParser::splitMnemonic(StringRef Mnemonic,
         Mnemonic == "fmrs" || Mnemonic == "fsqrts" || Mnemonic == "fsubs" ||
         Mnemonic == "fsts" || Mnemonic == "fcpys" || Mnemonic == "fdivs" ||
         Mnemonic == "fmuls" || Mnemonic == "fcmps" || Mnemonic == "fcmpzs" ||
-        Mnemonic == "vfms" || Mnemonic == "vfnms" ||
+        Mnemonic == "vfms" || Mnemonic == "vfnms" || Mnemonic == "fconsts" ||
         (Mnemonic == "movs" && isThumb()))) {
     Mnemonic = Mnemonic.slice(0, Mnemonic.size() - 1);
     CarrySetting = true;
diff --git a/test/MC/ARM/fconst.s b/test/MC/ARM/fconst.s
new file mode 100644
index 0000000..e2c1b39
--- /dev/null
+++ b/test/MC/ARM/fconst.s
@@ -0,0 +1,22 @@
+@ RUN: llvm-mc -mcpu=cortex-a8 -triple armv7-apple-darwin -show-encoding < %s | FileCheck %s
+
+@ fconstd/fconsts aliases
+        fconsts s4, #0x0
+        fconsts s4, #0x70
+        fconstd d3, #0x0
+        fconstd d3, #0x70
+
+        fconstsne s5, #0x1
+        fconstsgt s5, #0x20
+        fconstdlt d2, #0x3
+        fconstdge d2, #0x40
+
+@ CHECK: vmov.f32        s4, #2.000000e+00 @ encoding: [0x00,0x2a,0xb0,0xee]
+@ CHECK: vmov.f32        s4, #1.000000e+00 @ encoding: [0x00,0x2a,0xb7,0xee]
+@ CHECK: vmov.f64        d3, #2.000000e+00 @ encoding: [0x00,0x3b,0xb0,0xee]
+@ CHECK: vmov.f64        d3, #1.000000e+00 @ encoding: [0x00,0x3b,0xb7,0xee]
+
+@ CHECK: vmovne.f32      s5, #2.125000e+00 @ encoding: [0x01,0x2a,0xf0,0x1e]
+@ CHECK: vmovgt.f32      s5, #8.000000e+00 @ encoding: [0x00,0x2a,0xf2,0xce]
+@ CHECK: vmovlt.f64      d2, #2.375000e+00 @ encoding: [0x03,0x2b,0xb0,0xbe]
+@ CHECK: vmovge.f64      d2, #1.250000e-01 @ encoding: [0x00,0x2b,0xb4,0xae]
diff --git a/test/MC/ARM/fp-const-errors.s b/test/MC/ARM/fp-const-errors.s
new file mode 100644
index 0000000..2a68ddb
--- /dev/null
+++ b/test/MC/ARM/fp-const-errors.s
@@ -0,0 +1,22 @@
+@ RUN: not llvm-mc -mcpu=cortex-a8 -triple armv7-none-linux-gnueabi < %s 2>&1 | FileCheck %s
+
+@ Test for floating point constants that are out of the 8-bit encoded value range
+vmov.f32 s2, #32.0
+@ CHECK: error: invalid operand for instruction
+
+vmov.f64 d2, #32.0
+@ CHECK: error: invalid operand for instruction
+
+@ Test that vmov.f instructions do not accept an 8-bit encoded float as an operand
+vmov.f32 s1, #0x70
+@ CHECK: error: invalid floating point immediate
+
+vmov.f64 d2, #0x70
+@ CHECK: error: invalid floating point immediate
+
+@ Test that fconst instructions do not accept a float constant as an operand
+fconsts s1, #1.0
+@ CHECK: error: invalid floating point immediate
+
+fconstd d2, #1.0
+@ CHECK: error: invalid floating point immediate
diff --git a/test/MC/ARM/simple-fp-encoding.s b/test/MC/ARM/simple-fp-encoding.s
index d840e9c..539dd2c 100644
--- a/test/MC/ARM/simple-fp-encoding.s
+++ b/test/MC/ARM/simple-fp-encoding.s
@@ -395,3 +395,46 @@
 
 @ CHECK: vmov.i32	d4, #0x0        @ encoding: [0x10,0x40,0x80,0xf2]
 @ CHECK: vmov.i32	d4, #0x42000000 @ encoding: [0x12,0x46,0x84,0xf2]
+
+@ Test encoding of floating point constants for vmov functions
+@ vfp3
+         vmov.f32 s5, #1.0
+         vmov.f32 s5, #0.125
+         vmov.f32 s5, #-1.875
+         vmov.f32 s5, #-0.59375
+
+         vmov.f64 d6, #1.0
+         vmov.f64 d6, #0.125
+         vmov.f64 d6, #-1.875
+         vmov.f64 d6, #-0.59375
+
+@ neon
+         vmov.f32 d7, #1.0
+         vmov.f32 d7, #0.125
+         vmov.f32 d7, #-1.875
+         vmov.f32 d7, #-0.59375
+
+         vmov.f32 q8, #1.0
+         vmov.f32 q8, #0.125
+         vmov.f32 q8, #-1.875
+         vmov.f32 q8, #-0.59375
+
+@ CHECK: vmov.f32        s5, #1.000000e+00 @ encoding: [0x00,0x2a,0xf7,0xee]
+@ CHECK: vmov.f32        s5, #1.250000e-01 @ encoding: [0x00,0x2a,0xf4,0xee]
+@ CHECK: vmov.f32        s5, #-1.875000e+00 @ encoding: [0x0e,0x2a,0xff,0xee]
+@ CHECK: vmov.f32        s5, #-5.937500e-01 @ encoding: [0x03,0x2a,0xfe,0xee]
+
+@ CHECK: vmov.f64        d6, #1.000000e+00 @ encoding: [0x00,0x6b,0xb7,0xee]
+@ CHECK: vmov.f64        d6, #1.250000e-01 @ encoding: [0x00,0x6b,0xb4,0xee]
+@ CHECK: vmov.f64        d6, #-1.875000e+00 @ encoding: [0x0e,0x6b,0xbf,0xee]
+@ CHECK: vmov.f64        d6, #-5.937500e-01 @ encoding: [0x03,0x6b,0xbe,0xee]
+
+@ CHECK: vmov.f32        d7, #1.000000e+00 @ encoding: [0x10,0x7f,0x87,0xf2]
+@ CHECK: vmov.f32        d7, #1.250000e-01 @ encoding: [0x10,0x7f,0x84,0xf2]
+@ CHECK: vmov.f32        d7, #-1.875000e+00 @ encoding: [0x1e,0x7f,0x87,0xf3]
+@ CHECK: vmov.f32        d7, #-5.937500e-01 @ encoding: [0x13,0x7f,0x86,0xf3]
+
+@ CHECK: vmov.f32        q8, #1.000000e+00 @ encoding: [0x50,0x0f,0xc7,0xf2]
+@ CHECK: vmov.f32        q8, #1.250000e-01 @ encoding: [0x50,0x0f,0xc4,0xf2]
+@ CHECK: vmov.f32        q8, #-1.875000e+00 @ encoding: [0x5e,0x0f,0xc7,0xf3]
+@ CHECK: vmov.f32        q8, #-5.937500e-01 @ encoding: [0x53,0x0f,0xc6,0xf3]
-- 
1.7.8.3

More information about the llvm-commits mailing list