[ARM] Remove pre-UAL FLDM/FSTM aliases

[olista01]

Summary

These are pre-UAL syntax, and we don't support any other pre-UAL instructions, with the exception of FLDMX/FSTMX, which don't have a UAL equivalent. Therefore there's no reason to keep them or their AsmParser hacks around.

[rengolin]

We don't want pre-UAL syntax. I know these were there before, but moving them up into table-gen encourages people to further add pre-UAL support and I believe we should stay well away from it.

Is there any reason why we don't just remove it?

[olista01]

+Saleem, who added these aliases in 2013.

Do you remember why these aliases were added, but not any other pre-UAL VFP instructions? There is a section in the v7AR reference manual calling out the FLDMX and FSTMX instructions as not having a UAL syntax, while still having a defined (but deprecated) baheviour, but this patch doesn't touch them.

[compnerd]

IIRC it had something to do with not having the equivalent mnemonics in UAL. If there is an equivalent, I don't think that we need to bother with the addition of the support for the pre-UAL syntax. Its been long enough, and most of the translation is relatively easy.

[olista01]

I've double-checked, and the FLDMX and FSTMX are the only pre-UAL VFP instructions that don't have a UAL syntax, so I've removed all of the FLDMS anbd FLDMD aliases.

This still removes all of the hacks in the AsmParser, so the diagnostics for the register list operand in FLDMX and FSTMX are now the standard ones for the dpr_reglist operand class, as used by the UAL instructions.

[rengolin]

Adding @joerg, who sometimes deals in old Entish ARM dialects.

@compnerd Did we clear the whole libunwind of those mnemonics? Was there any other reason why we had them?

I'm happy to see those go, but I fear someone will eventually cry foul that we "broke their code".

Essentially, if @compnerd / @joerg are happy, so am I. :)

[olista01]

Ping, the dependency (D39195) is now committed.

[rengolin]

LGTM, thanks!

[ikudrin]

Hi @rengolin

In D39196#904887, @rengolin wrote:

I fear someone will eventually cry foul that we "broke their code".

You were right. I afraid that this change breaks building some Android code. See https://android.googlesource.com/platform/frameworks/av, and, for example, the android-8.1.0_r9 tag, media/libstagefright/codecs/aacenc/src/asm/ARMV7/*.S. These files use fstmfdd and fldmfdd and are failed to compile now. They are already removed from the master branch (merge commit 69d60c1372, 17-Oct-2017), however, I believe that we have to be able to compile the existing versions.

lib/Target/ARM/ARMInstrVFP.td

def VLSTM : AXSI4<(outs), (ins GPRnopc:$Rn, pred:$p), IndexModeNone,
     let Inst{7-0}   = 0;
     let mayStore    = 1;
 }
 
 
 // FLDM/FSTM - Load / Store multiple single / double precision registers for
 // pre-ARMv6 cores.
 // These instructions are deprecated!
-def : VFP2MnemonicAlias<"fldmias", "vldmia">;
-def : VFP2MnemonicAlias<"fldmdbs", "vldmdb">;
-def : VFP2MnemonicAlias<"fldmeas", "vldmdb">;
-def : VFP2MnemonicAlias<"fldmfds", "vldmia">;
-def : VFP2MnemonicAlias<"fldmiad", "vldmia">;
-def : VFP2MnemonicAlias<"fldmdbd", "vldmdb">;
-def : VFP2MnemonicAlias<"fldmead", "vldmdb">;
-def : VFP2MnemonicAlias<"fldmfdd", "vldmia">;
-
-def : VFP2MnemonicAlias<"fstmias", "vstmia">;
-def : VFP2MnemonicAlias<"fstmdbs", "vstmdb">;
-def : VFP2MnemonicAlias<"fstmeas", "vstmia">;
-def : VFP2MnemonicAlias<"fstmfds", "vstmdb">;
-def : VFP2MnemonicAlias<"fstmiad", "vstmia">;
-def : VFP2MnemonicAlias<"fstmdbd", "vstmdb">;
-def : VFP2MnemonicAlias<"fstmead", "vstmia">;
-def : VFP2MnemonicAlias<"fstmfdd", "vstmdb">;
+class VFPLdStMultipleAlias<string mnemonic, string writeback,
+                           Instruction new_inst, Operand list_type>
+      : InstAlias<!strconcat(mnemonic, "${p}\t$Rn", writeback, ", $reglist"),
+                 (new_inst GPR:$Rn, pred:$p, list_type:$reglist), 0>;
+def : VFPLdStMultipleAlias<"fldmias", "",  VLDMSIA,     spr_reglist>;
+def : VFPLdStMultipleAlias<"fldmfds", "",  VLDMSIA,     spr_reglist>;
+def : VFPLdStMultipleAlias<"fldmias", "!", VLDMSIA_UPD, spr_reglist>;
+def : VFPLdStMultipleAlias<"fldmfds", "!", VLDMSIA_UPD, spr_reglist>;
+def : VFPLdStMultipleAlias<"fldmdbs", "!", VLDMSDB_UPD, spr_reglist>;
+def : VFPLdStMultipleAlias<"fldmeas", "!", VLDMSDB_UPD, spr_reglist>;
+
+def : VFPLdStMultipleAlias<"fldmiad", "",  VLDMDIA,     dpr_reglist>;
+def : VFPLdStMultipleAlias<"fldmfdd", "",  VLDMDIA,     dpr_reglist>;
+def : VFPLdStMultipleAlias<"fldmiad", "!", VLDMDIA_UPD, dpr_reglist>;
+def : VFPLdStMultipleAlias<"fldmfdd", "!", VLDMDIA_UPD, dpr_reglist>;
+def : VFPLdStMultipleAlias<"fldmdbd", "!", VLDMDDB_UPD, dpr_reglist>;
+def : VFPLdStMultipleAlias<"fldmead", "!", VLDMDDB_UPD, dpr_reglist>;
+
+def : VFPLdStMultipleAlias<"fstmias", "",  VSTMSIA,     spr_reglist>;
+def : VFPLdStMultipleAlias<"fstmeas", "",  VSTMSIA,     spr_reglist>;
+def : VFPLdStMultipleAlias<"fstmias", "!", VSTMSIA_UPD, spr_reglist>;
+def : VFPLdStMultipleAlias<"fstmeas", "!", VSTMSIA_UPD, spr_reglist>;
+def : VFPLdStMultipleAlias<"fstmdbs", "!", VSTMSDB_UPD, spr_reglist>;
+def : VFPLdStMultipleAlias<"fstmfds", "!", VSTMSDB_UPD, spr_reglist>;
+
+def : VFPLdStMultipleAlias<"fstmiad", "",  VSTMDIA,     dpr_reglist>;
+def : VFPLdStMultipleAlias<"fstmaed", "",  VSTMDIA,     dpr_reglist>;
+def : VFPLdStMultipleAlias<"fstmiad", "!", VSTMDIA_UPD, dpr_reglist>;
+def : VFPLdStMultipleAlias<"fstmead", "!", VSTMDIA_UPD, dpr_reglist>;
+def : VFPLdStMultipleAlias<"fstmdbd", "!", VSTMDDB_UPD, dpr_reglist>;
+def : VFPLdStMultipleAlias<"fstmfdd", "!", VSTMDDB_UPD, dpr_reglist>;
 
 def : InstAlias<"vpush${p} $r", (VSTMDDB_UPD SP, pred:$p, dpr_reglist:$r), 0>,
                 Requires<[HasVFP2]>;
 def : InstAlias<"vpush${p} $r", (VSTMSDB_UPD SP, pred:$p, spr_reglist:$r), 0>,
                 Requires<[HasVFP2]>;
 def : InstAlias<"vpop${p} $r",  (VLDMDIA_UPD SP, pred:$p, dpr_reglist:$r), 0>,
                 Requires<[HasVFP2]>;
 def : InstAlias<"vpop${p} $r",  (VLDMSIA_UPD SP, pred:$p, spr_reglist:$r), 0>,

lib/Target/ARM/AsmParser/ARMAsmParser.cpp

if (Mnemonic.endswith("s") &&
         Mnemonic == "vcls" || Mnemonic == "vmls" || Mnemonic == "vmrs" ||
         Mnemonic == "vnmls" || Mnemonic == "vqabs" || Mnemonic == "vrecps" ||
         Mnemonic == "vrsqrts" || Mnemonic == "srs" || Mnemonic == "flds" ||
         Mnemonic == "fmrs" || Mnemonic == "fsqrts" || Mnemonic == "fsubs" ||
         Mnemonic == "fsts" || Mnemonic == "fcpys" || Mnemonic == "fdivs" ||
         Mnemonic == "fmuls" || Mnemonic == "fcmps" || Mnemonic == "fcmpzs" ||
         Mnemonic == "vfms" || Mnemonic == "vfnms" || Mnemonic == "fconsts" ||
         Mnemonic == "bxns" || Mnemonic == "blxns" ||
-        (Mnemonic == "movs" && isThumb()))) {
+        (Mnemonic == "movs" && isThumb()) ||
+        Mnemonic.startswith("fldm") || Mnemonic.startswith("fstm"))) {
     Mnemonic = Mnemonic.slice(0, Mnemonic.size() - 1);
     CarrySetting = true;
   }
 
   // The "cps" instruction can have a interrupt mode operand which is glued into
   // the mnemonic. Check if this is the case, split it and parse the imod op
   if (Mnemonic.startswith("cps")) {
     // Split out any imod code.

 // a literal string token the way it is now.
 static bool doesIgnoreDataTypeSuffix(StringRef Mnemonic, StringRef DT) {
   return Mnemonic.startswith("vldm") || Mnemonic.startswith("vstm");
 }
 
 static void applyMnemonicAliases(StringRef &Mnemonic, uint64_t Features,
                                  unsigned VariantID);
 
-static bool RequiresVFPRegListValidation(StringRef Inst,
-                                         bool &AcceptSinglePrecisionOnly,
-                                         bool &AcceptDoublePrecisionOnly) {
-  if (Inst.size() < 7)
-    return false;
-
-  if (Inst.startswith("fldm") || Inst.startswith("fstm")) {
-    StringRef AddressingMode = Inst.substr(4, 2);
-    if (AddressingMode == "ia" || AddressingMode == "db" ||
-        AddressingMode == "ea" || AddressingMode == "fd") {
-      AcceptSinglePrecisionOnly = Inst[6] == 's';
-      AcceptDoublePrecisionOnly = Inst[6] == 'd' || Inst[6] == 'x';
-      return true;
-    }
-  }
-
-  return false;
-}
-
 // The GNU assembler has aliases of ldrd and strd with the second register
 // omitted. We don't have a way to do that in tablegen, so fix it up here.
 //
 // We have to be careful to not emit an invalid Rt2 here, because the rest of
 // the assmebly parser could then generate confusing diagnostics refering to
 // it. If we do find anything that prevents us from doing the transformation we
 // bail out, and let the assembly parser report an error on the instruction as
 // it is written.
Operands.insert(
       ARMOperand::CreateReg(PairedReg, Op2.getStartLoc(), Op2.getEndLoc()));
   return;
 }
 
 /// Parse an arm instruction mnemonic followed by its operands.
 bool ARMAsmParser::ParseInstruction(ParseInstructionInfo &Info, StringRef Name,
                                     SMLoc NameLoc, OperandVector &Operands) {
   MCAsmParser &Parser = getParser();
-  // FIXME: Can this be done via tablegen in some fashion?
-  bool RequireVFPRegisterListCheck;
-  bool AcceptSinglePrecisionOnly;
-  bool AcceptDoublePrecisionOnly;
-  RequireVFPRegisterListCheck =
-    RequiresVFPRegListValidation(Name, AcceptSinglePrecisionOnly,
-                                 AcceptDoublePrecisionOnly);
 
   // Apply mnemonic aliases before doing anything else, as the destination
   // mnemonic may include suffices and we want to handle them normally.
   // The generic tblgen'erated code does this later, at the start of
   // MatchInstructionImpl(), but that's too late for aliases that include
   // any sort of suffix.
   uint64_t AvailableFeatures = getAvailableFeatures();
   unsigned AssemblerDialect = getParser().getAssemblerDialect();
while (parseOptionalToken(AsmToken::Comma)) {
         return true;
       }
     }
   }
 
   if (parseToken(AsmToken::EndOfStatement, "unexpected token in argument list"))
     return true;
 
-  if (RequireVFPRegisterListCheck) {
-    ARMOperand &Op = static_cast<ARMOperand &>(*Operands.back());
-    if (AcceptSinglePrecisionOnly && !Op.isSPRRegList())
-      return Error(Op.getStartLoc(),
-                   "VFP/Neon single precision register expected");
-    if (AcceptDoublePrecisionOnly && !Op.isDPRRegList())
-      return Error(Op.getStartLoc(),
-                   "VFP/Neon double precision register expected");
-  }
-
   tryConvertingToTwoOperandForm(Mnemonic, CarrySetting, Operands);
 
   // Some instructions, mostly Thumb, have forms for the same mnemonic that
   // do and don't have a cc_out optional-def operand. With some spot-checks
   // of the operand list, we can figure out which variant we're trying to
   // parse and adjust accordingly before actually matching. We shouldn't ever
   // try to remove a cc_out operand that was explicitly set on the
   // mnemonic, of course (CarrySetting == true). Reason number #317 the

test/MC/ARM/vfp-aliases-diagnostics.s

aliases:
 
   fstmeax sp!, {s0}
   fldmfdx sp!, {s0}
 
   fstmfdx sp!, {s0}
   fldmeax sp!, {s0}
 
 @ CHECK-LABEL: aliases
-@ CHECK: error: VFP/Neon double precision register expected
+@ CHECK: error: operand must be a list of registers in range [d0, d31]
 @ CHECK:  fstmfdd sp!, {s0}
 @ CHECK:                     ^
-@ CHECK: error: VFP/Neon double precision register expected
+@ CHECK: error: operand must be a list of registers in range [d0, d31]
 @ CHECK:  fstmead sp!, {s0}
 @ CHECK:                     ^
-@ CHECK: error: VFP/Neon double precision register expected
+@ CHECK: error: operand must be a list of registers in range [d0, d31]
 @ CHECK:  fstmdbd sp!, {s0}
 @ CHECK:                     ^
-@ CHECK: error: VFP/Neon double precision register expected
+@ CHECK: error: operand must be a list of registers in range [d0, d31]
 @ CHECK:  fstmiad sp!, {s0}
 @ CHECK:                     ^
-@ CHECK: error: VFP/Neon single precision register expected
+@ CHECK: error: operand must be a list of registers in range [s0, s31]
 @ CHECK:  fstmfds sp!, {d0}
 @ CHECK:                     ^
-@ CHECK: error: VFP/Neon single precision register expected
+@ CHECK: error: operand must be a list of registers in range [s0, s31]
 @ CHECK:  fstmeas sp!, {d0}
 @ CHECK:                     ^
-@ CHECK: error: VFP/Neon single precision register expected
+@ CHECK: error: operand must be a list of registers in range [s0, s31]
 @ CHECK:  fstmdbs sp!, {d0}
 @ CHECK:                     ^
-@ CHECK: error: VFP/Neon single precision register expected
+@ CHECK: error: operand must be a list of registers in range [s0, s31]
 @ CHECK:  fstmias sp!, {d0}
 @ CHECK:                     ^
 
-@ CHECK: error: VFP/Neon single precision register expected
+@ CHECK: error: operand must be a list of registers in range [s0, s31]
 @ CHECK:  fldmias sp!, {d0}
 @ CHECK:                     ^
-@ CHECK: error: VFP/Neon single precision register expected
+@ CHECK: error: operand must be a list of registers in range [s0, s31]
 @ CHECK:  fldmdbs sp!, {d0}
 @ CHECK:                     ^
-@ CHECK: error: VFP/Neon single precision register expected
+@ CHECK: error: operand must be a list of registers in range [s0, s31]
 @ CHECK:  fldmeas sp!, {d0}
 @ CHECK:                     ^
-@ CHECK: error: VFP/Neon single precision register expected
+@ CHECK: error: operand must be a list of registers in range [s0, s31]
 @ CHECK:  fldmfds sp!, {d0}
 @ CHECK:                     ^
-@ CHECK: error: VFP/Neon double precision register expected
+@ CHECK: error: operand must be a list of registers in range [d0, d31]
 @ CHECK:  fldmiad sp!, {s0}
 @ CHECK:                     ^
-@ CHECK: error: VFP/Neon double precision register expected
+@ CHECK: error: operand must be a list of registers in range [d0, d31]
 @ CHECK:  fldmdbd sp!, {s0}
 @ CHECK:                     ^
-@ CHECK: error: VFP/Neon double precision register expected
+@ CHECK: error: operand must be a list of registers in range [d0, d31]
 @ CHECK:  fldmead sp!, {s0}
 @ CHECK:                     ^
-@ CHECK: error: VFP/Neon double precision register expected
+@ CHECK: error: operand must be a list of registers in range [d0, d31]
 @ CHECK:  fldmfdd sp!, {s0}
 @ CHECK:                     ^
 
-@ CHECK: error: VFP/Neon double precision register expected
+@ CHECK: error: operand must be a list of registers in range [d0, d31]
 @ CHECK:  fstmeax sp!, {s0}
 @ CHECK:                     ^
-@ CHECK: error: VFP/Neon double precision register expected
+@ CHECK: error: operand must be a list of registers in range [d0, d31]
 @ CHECK:  fldmfdx sp!, {s0}
 @ CHECK:                     ^
 
-@ CHECK: error: VFP/Neon double precision register expected
+@ CHECK: error: operand must be a list of registers in range [d0, d31]
 @ CHECK:  fstmfdx sp!, {s0}
 @ CHECK:                     ^
-@ CHECK: error: VFP/Neon double precision register expected
+@ CHECK: error: operand must be a list of registers in range [d0, d31]
 @ CHECK:  fldmeax sp!, {s0}
 @ CHECK:                     ^
 
   fstmiaxcs r0, {s0}
   fstmiaxhs r0, {s0}
   fstmiaxls r0, {s0}
   fstmiaxvs r0, {s0}
-@ CHECK: error: VFP/Neon double precision register expected
+@ CHECK: error: operand must be a list of registers in range [d0, d31]
 @ CHECK:  fstmiaxcs r0, {s0}
 @ CHECK:                      ^
-@ CHECK: error: VFP/Neon double precision register expected
+@ CHECK: error: operand must be a list of registers in range [d0, d31]
 @ CHECK:  fstmiaxhs r0, {s0}
 @ CHECK:                      ^
-@ CHECK: error: VFP/Neon double precision register expected
+@ CHECK: error: operand must be a list of registers in range [d0, d31]
 @ CHECK:  fstmiaxls r0, {s0}
 @ CHECK:                      ^
-@ CHECK: error: VFP/Neon double precision register expected
+@ CHECK: error: operand must be a list of registers in range [d0, d31]
 @ CHECK:  fstmiaxvs r0, {s0}
 @ CHECK:                      ^