[PATCH] D36084: [globalisel][tablegen] Support zero-instruction emission.

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added inline comments.


================
Comment at: utils/TableGen/GlobalISelEmitter.cpp:2293
                         to_string(Src->getExtTypes().size()) + " def(s) vs " +
                         to_string(DstI.Operands.NumDefs) + " def(s))");
 
----------------
dsanders wrote:
> rovka wrote:
> > Would it make sense to move these 2 checks higher up, as a quick exit? (So we don't createAndImport the Src only to discover that Dst wasn't an instruction in the first place).
> We could do that. The only reason it comes after the matcher is to keep a boundary between the code that handles the matcher and the code that handles the renderer. The two were tangled together in the early versions of GlobalISelEmitter.cpp.
I just went to do this and I don't think it can be moved up without significant changes. The `Dst->getOperator()` on line 2285 requires that `Dst->isLeaf()` is false so it must either come after the if-statement on line 2255 or be wrapped in a `if (!Dst->isLeaf()) {}`. If we choose to move the if-statement on line 2255 up as well then we're creating the renderer before the matcher which will break `getOperand(StringRef)` because the variables (`defineInsnVar()`) wont be defined at that point. If we chose to wrap it in an `if (!Dst->isLeaf())` and move it past line 2255's if-statement then we have a use-beyond-scope issue to fix and fixing that requires allowing DstOp to be null and rewriting DstI to be a (possibly-null) pointer which will impact the `createAndImportInstructionRenderer()` function and those it calls.


https://reviews.llvm.org/D36084