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

[Daniel Sanders via Phabricator via llvm-commits]

dsanders 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:
> 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?
Sorry, forgot to come back to this.

There's the emit*() functions but this patch already rewrites the code that might be affected so I think we're fine.


Repository:
  rL LLVM

https://reviews.llvm.org/D39034