[ARM] Don't omit non-default predication code

[olista01]

Summary

This was causing the (invalid) predicated versions of the NEON VRINTX and VRINTZ instructions to be accepted, with the condition code being ignored.

Also, there is no NEON VRINTR instruction, so that part of the check was not necessary.

[olista01]

Ping

[rengolin]

lib/Target/ARM/AsmParser/ARMAsmParser.cpp

5810 ↗

(On Diff #119884)

update comment?

test/MC/ARM/invalid-neon-v8.s

76 ↗

(On Diff #119884)

do we have tests for when it's valid? for all three R,X,Z variants?

[olista01]

test/MC/ARM/invalid-neon-v8.s

76 ↗

(On Diff #119884)

Yes, there are tests for the conditional and unconditional VFP versions in test/MC/ARM/fp-armv8.s and test/MC/ARM/thumb-fp-armv8.s, and the unconditional NEON case in test/MC/ARM/neon-v8.s.

[rengolin]

Ok, with the updated comment, LGTM. Thanks!

[olista01]

Fix comment.

llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp

if (isThumb() && (Mnemonic == "add" || Mnemonic == "sub") &&
         static_cast<ARMOperand &>(*Operands[5]).isImm())))
     return true;
 
   return false;
 }
 
 bool ARMAsmParser::shouldOmitPredicateOperand(StringRef Mnemonic,
                                               OperandVector &Operands) {
-  // VRINT{Z, R, X} have a predicate operand in VFP, but not in NEON
+  // VRINT{Z, X} have a predicate operand in VFP, but not in NEON
   unsigned RegIdx = 3;
-  if ((Mnemonic == "vrintz" || Mnemonic == "vrintx" || Mnemonic == "vrintr") &&
+  if ((Mnemonic == "vrintz" || Mnemonic == "vrintx") &&
       (static_cast<ARMOperand &>(*Operands[2]).getToken() == ".f32" ||
        static_cast<ARMOperand &>(*Operands[2]).getToken() == ".f16")) {
     if (static_cast<ARMOperand &>(*Operands[3]).isToken() &&
         (static_cast<ARMOperand &>(*Operands[3]).getToken() == ".f32" ||
          static_cast<ARMOperand &>(*Operands[3]).getToken() == ".f16"))
       RegIdx = 4;
 
     if (static_cast<ARMOperand &>(*Operands[RegIdx]).isReg() &&
bool ARMAsmParser::ParseInstruction(ParseInstructionInfo &Info, StringRef Name,
   // mnemonic, of course (CarrySetting == true). Reason number #317 the
   // table driven matcher doesn't fit well with the ARM instruction set.
   if (!CarrySetting && shouldOmitCCOutOperand(Mnemonic, Operands))
     Operands.erase(Operands.begin() + 1);
 
   // Some instructions have the same mnemonic, but don't always
   // have a predicate. Distinguish them here and delete the
   // predicate if needed.
-  if (shouldOmitPredicateOperand(Mnemonic, Operands))
+  if (PredicationCode == ARMCC::AL &&
+      shouldOmitPredicateOperand(Mnemonic, Operands))
     Operands.erase(Operands.begin() + 1);
 
   // ARM mode 'blx' need special handling, as the register operand version
   // is predicable, but the label operand version is not. So, we can't rely
   // on the Mnemonic based checking to correctly figure out when to put
   // a k_CondCode operand in the list. If we're trying to match the label
   // version, remove the k_CondCode operand here.
   if (!isThumb() && Mnemonic == "blx" && Operands.size() == 3 &&

llvm/trunk/test/MC/ARM/invalid-fp-armv8.s

 @ V8: note: operand must be a register in range [d0, d31]
 @ V8: note: invalid operand for instruction
 vcvtthi.f16.f64 q0, d3
 @ V8: error: operand must be a register in range [s0, s31]
 
 vrintrlo.f32.f32 d3, q0
 @ V8: error: invalid instruction
 vrintxcs.f32.f32 d3, d0
-@ V8: error: instruction requires: NEON
+@ V8: error: invalid instruction
 
 vrinta.f64.f64 s3, q0
 @ V8: error: invalid instruction
 vrintn.f32.f32 d3, d0
 @ V8: error: instruction requires: NEON
 vrintp.f32 q3, q0
 @ V8: error: instruction requires: NEON
 vrintmlt.f32 q3, q0
 @ V8: error: instruction 'vrintm' is not predicable, but condition code specified

llvm/trunk/test/MC/ARM/invalid-neon-v8.s

 @ CHECK: error: instruction 'sha256su1' is not predicable, but condition code specified
 
 vmull.p64 q0, s1, s3
 @ CHECK: error: invalid instruction
 vmull.p64 s1, d2, d3
 @ CHECK: error: operand must be a register in range [q0, q15]
 vmullge.p64 q0, d16, d17
 @ CHECK: error: instruction 'vmull' is not predicable, but condition code specified
+
+// These instructions are predicable in VFP but not in NEON
+vrintzeq.f32 d0, d1
+vrintxgt.f32 d0, d1
+@ CHECK: error: invalid operand for instruction
+@ CHECK: error: invalid operand for instruction