[AArch64] Fix handling of zero immediate in fmov instructions

[john.brawn]

Summary

Currently fmov #0 with a vector destination is handle incorrectly and results in fmov #-1.9375 being emitted but should instead give an error. This is due to the way we cope with fmov #0 with a scalar destination being an alias of fmov zr, so fix this by actually doing it through an alias.

[john.brawn]

Add diff context.

[john.brawn]

Accidentally used the wrong diff, attach the correct one.

[rengolin]

lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

2123

Is this where the Parser.lex() from D31948 ended up?

2125

Why two tokens?

[john.brawn]

lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

2125

Because that's what the MatchTable in AArch64GenAsmMatcher.inc expects for the #0.0 in the fmov InstAlias. After a bit of poking around it looks like this is because the AsmParserVariants defined in AArch64.td sets '.' to be a break character which starts a new token, probably so that the instruction qualifiers (which start with a '.') get treated as operands not as part of the mnemonic.

[john.brawn]

Ping.

[rengolin]

LGTM. Thanks!

lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

2125

Right, I expected as much.

lib/Target/AArch64/AArch64InstrInfo.td

 
 // Add pseudo ops for FMOV 0 so we can mark them as isReMaterializable
 let isReMaterializable = 1, isCodeGenOnly = 1, isAsCheapAsAMove = 1 in {
 def FMOVS0 : Pseudo<(outs FPR32:$Rd), (ins), [(set f32:$Rd, (fpimm0))]>,
     Sched<[WriteF]>;
 def FMOVD0 : Pseudo<(outs FPR64:$Rd), (ins), [(set f64:$Rd, (fpimm0))]>,
     Sched<[WriteF]>;
 }
+// Similarly add aliases
+def : InstAlias<"fmov $Rd, #0.0", (FMOVWHr FPR16:$Rd, WZR), 0>,
+    Requires<[HasFullFP16]>;
+def : InstAlias<"fmov $Rd, #0.0", (FMOVWSr FPR32:$Rd, WZR), 0>;
+def : InstAlias<"fmov $Rd, #0.0", (FMOVXDr FPR64:$Rd, XZR), 0>;
 
 //===----------------------------------------------------------------------===//
 // Floating point conversion instruction.
 //===----------------------------------------------------------------------===//
 
 defm FCVT : FPConversion<"fcvt">;
 
 //===----------------------------------------------------------------------===//

lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

if (Tok.is(AsmToken::Real) || Tok.is(AsmToken::Integer)) {
     } else {
       APFloat RealVal(APFloat::IEEEdouble(), Tok.getString());
       if (isNegative)
         RealVal.changeSign();
 
       uint64_t IntVal = RealVal.bitcastToAPInt().getZExtValue();
       Val = AArch64_AM::getFP64Imm(APInt(64, IntVal));
 
-      // Check for out of range values. As an exception, we let Zero through,
-      // as we handle that special case in post-processing before matching in
-      // order to use the zero register for it.
-      if (Val == -1 && !RealVal.isPosZero()) {
+      // Check for out of range values. As an exception we let Zero through,
+      // but as tokens instead of an FPImm so that it can be matched by the
+      // appropriate alias if one exists.
+      if (RealVal.isPosZero()) {
+        Parser.Lex(); // Eat the token.
+        Operands.push_back(AArch64Operand::CreateToken("#0", false, S, getContext()));
+        Operands.push_back(AArch64Operand::CreateToken(".0", false, S, getContext()));
+        return MatchOperand_Success;
+      } else if (Val == -1) {
         TokError("expected compatible register or floating-point constant");
         return MatchOperand_ParseFail;
       }
     }
     Parser.Lex(); // Eat the token.
     Operands.push_back(AArch64Operand::CreateFPImm(Val, S, getContext()));
     return MatchOperand_Success;
   }
if (Op.isReg() &&
       if (Op.isReg()) {
         unsigned Reg = getWRegFromXReg(Op.getReg());
         Operands[1] = AArch64Operand::CreateReg(Reg, false, Op.getStartLoc(),
                                                 Op.getEndLoc(), getContext());
       }
     }
   }
 
-  // Yet another horrible hack to handle FMOV Rd, #0.0 using [WX]ZR.
-  if (NumOperands == 3 && Tok == "fmov") {
-    AArch64Operand &RegOp = static_cast<AArch64Operand &>(*Operands[1]);
-    AArch64Operand &ImmOp = static_cast<AArch64Operand &>(*Operands[2]);
-    if (RegOp.isReg() && ImmOp.isFPImm() && ImmOp.getFPImm() == (unsigned)-1) {
-      unsigned zreg =
-          !AArch64MCRegisterClasses[AArch64::FPR64RegClassID].contains(
-              RegOp.getReg())
-              ? AArch64::WZR
-              : AArch64::XZR;
-      Operands[2] = AArch64Operand::CreateReg(zreg, false, Op.getStartLoc(),
-                                              Op.getEndLoc(), getContext());
-    }
-  }
-
   MCInst Inst;
   // First try to match against the secondary set of tables containing the
   // short-form NEON instructions (e.g. "fadd.2s v0, v1, v2").
   unsigned MatchResult =
       MatchInstructionImpl(Operands, Inst, ErrorInfo, MatchingInlineAsm, 1);
 
   // If that fails, try against the alternate table containing long-form NEON:
   // "fadd v0.2s, v1.2s, v2.2s"

test/MC/AArch64/basic-a64-diagnostics.s

 // CHECK-ERROR-NEXT:                     ^
 
         ;; Explicitly encoded value too large
         fmov s15, #0x100
 // CHECK-ERROR: error: encoded floating point value out of range
 // CHECK-ERROR-NEXT:           fmov s15, #0x100
 // CHECK-ERROR-NEXT:                     ^
 
-        ;; No particular reason, but a striking omission
-        fmov d0, #0.0
-// CHECK-ERROR-AARCH64: error: expected compatible register or floating-point constant
-// CHECK-ERROR-AARCH64-NEXT:           fmov d0, #0.0
-// CHECK-ERROR-AARCH64-NEXT:                    ^
+        ;; Not possible to fmov ZR to a whole vector
+        fmov v0.4s, #0.0
+// CHECK-ERROR: error: invalid operand for instruction
+// CHECK-ERROR-NEXT:           fmov v0.4s, #0.0
+// CHECK-ERROR-NEXT:                       ^
 
 //------------------------------------------------------------------------------
 // Floating-point <-> integer conversion
 //------------------------------------------------------------------------------
 
         fmov x3, v0.d[0]
         fmov v29.1d[1], x2
         fmov x7, v0.d[2]