[PATCH] D39034: [WIP][GlobalISel][TableGen] Optimize MatchTable for faster instruction selection

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet added inline comments.


================
Comment at: utils/TableGen/GlobalISelEmitter.cpp:3365-3367
+    // If this operand is free of constraints, rip it off.
+    if (OpMatcher.predicates_empty()) {
+      Matcher.pop_front();
----------------
qcolombet wrote:
> dsanders wrote:
> > qcolombet wrote:
> > > qcolombet wrote:
> > > > dsanders wrote:
> > > > > This doesn't look right. It's removing the whole instruction matcher if its first operand matcher runs out of predicates. There may be other operand matchers left which do have predicates.
> > > > Good catch, this check is supposed to check that OpMatcher was the last one!
> > > Actually, I've checked the implementation of InstructionMatcher::pop_front and it only does what I intended: removing the first operand.
> > > 
> > > Talking with Daniel offline there was something else to fix: getNumOperands, so that the related check remains correct (it was looking at the size of this list). 
> > Sorry about that.
> > 
> > There’s also the operand index which uses the position within this list
> Okay, looking.
Seems like InstMatcher::getOperand would already do the right thing (it checks that getOperandIndex == OpIdx) and AFAICT the uses of OperandMatcher::getOperandIndex are fine.
What else should I check?


Repository:
  rL LLVM

https://reviews.llvm.org/D39034