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

Diana Picus via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 11 01:56:00 PDT 2017

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


More information about the llvm-commits mailing list