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

[Diana Picus via Phabricator via llvm-commits]

rovka added a comment.

In https://reviews.llvm.org/D36084#830458, @dsanders wrote:

> I had a testcase for this one but it seems to have gone missing. I'll update with one shortly


LGTM if you add that test.



================
Comment at: utils/TableGen/GlobalISelEmitter.cpp:2293
                         to_string(Src->getExtTypes().size()) + " def(s) vs " +
                         to_string(DstI.Operands.NumDefs) + " def(s))");
 
----------------
dsanders wrote:
> 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.
Ok, it's fine as it is then.


================
Comment at: utils/TableGen/GlobalISelEmitter.cpp:2295
+    if (RCDef) {
+      // We need to replace the def and all it's uses with the specified
+      // operand. However, we must also insert COPY's wherever needed.
----------------
Typo: its


https://reviews.llvm.org/D36084