[PATCH] D25618: Check that emitted instructions meet their predicates on all targets except ARM, Mips, and X86.

[James Molloy via llvm-commits]

jmolloy added a comment.

This is a great idea, thanks for working on this!

My biggest concern is that there's very little information printed out for a developer to take action on. We absolutely need to have at least the instruction that failed verification printed, and ideally the expected and actual feature sets.

James



================
Comment at: lib/Target/AArch64/MCTargetDesc/AArch64MCCodeEmitter.cpp:175
+private:
+  uint64_t ComputeAvailableFeatures(const FeatureBitset &FB) const;
+  void verifyInstructionPredicates(const MCInst &MI,
----------------
lowerCamelCase


================
Comment at: utils/TableGen/AsmMatcherEmitter.cpp:1417
   // Build information about all of the AssemblerPredicates.
+
   const auto &SubtargetFeaturePairs = SubtargetFeatureInfo::getAll(Records);
----------------
Whitespace change


================
Comment at: utils/TableGen/CodeEmitterGen.cpp:347
+  o << "  assert((AvailableFeatures & RequiredFeatures[Inst.getOpcode()])\n"
+    << "             == RequiredFeatures[Inst.getOpcode()] &&\n"
+    << "         \"Attempting to emit an instruction that doesn't satisfy the required\"\n"
----------------
This absolutely needs to print out the offending instruction, and ideally the required features although as they're encoded in a bitset that's probably quite difficult.


https://reviews.llvm.org/D25618