[PATCH] D69653: [GlobalISel] Match table opt: fix a bug in matching num of operands

[Roman Tereshin via Phabricator via llvm-commits]

rtereshin created this revision.
rtereshin added reviewers: dsanders, qcolombet.
Herald added subscribers: llvm-commits, Petar.Avramovic, rovka.
Herald added a project: LLVM.

If there is a dag node with a variable number of operands that has at
least N operands (for some non-negative N), and multiple patterns with
that node with different number of operands, we would drop the number of
operands check in patterns with N operands, presumably because it's
guaranteed in such case that none of the per-operand checks will access
the operand list out-of-bounds.

Except semantically the check is about having exactly N operands, not at
least N operands, and a backend might rely on it to disambiguate
different patterns.

In this patch we change the condition on emitting the number of operands
check from "the instruction is not guaranteed to have at least as many
operands as are checked by the pattern being matched" to "the
instruction is not guaranteed to have a specific number of operands".

We're relying (still) on the rest of the CodeGenPatterns mechanics to
validate that the pattern itself doesn't try to access more operands
than there is in the instruction in cases when the instruction does have
fixed number of operands, and on the machine verifier to validate at
runtime that particular MIs like that satisfy the constraint as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69653

Files:
  llvm/utils/TableGen/GlobalISelEmitter.cpp


Index: llvm/utils/TableGen/GlobalISelEmitter.cpp
===================================================================
--- llvm/utils/TableGen/GlobalISelEmitter.cpp
+++ llvm/utils/TableGen/GlobalISelEmitter.cpp
@@ -1707,7 +1707,7 @@
   }
 
   StringRef getOpcode() const { return I->TheDef->getName(); }
-  unsigned getNumOperands() const { return I->Operands.size(); }
+  bool isVariadicNumOperands() const { return I->Operands.isVariadic; }
 
   StringRef getOperandType(unsigned OpIdx) const {
     return I->Operands[OpIdx].OperandType;
@@ -2273,7 +2273,7 @@
 
   Stash.push_back(predicates_pop_front());
   if (Stash.back().get() == &OpcMatcher) {
-    if (NumOperandsCheck && OpcMatcher.getNumOperands() < getNumOperands())
+    if (NumOperandsCheck && OpcMatcher.isVariadicNumOperands())
       Stash.emplace_back(
           new InstructionNumOperandsMatcher(InsnVarID, getNumOperands()));
     NumOperandsCheck = false;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D69653.227217.patch
Type: text/x-patch
Size: 927 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191031/80be0b80/attachment.bin>