[PATCH] D43409: [GISel]: Don't assert when constraining Registers which are uses if there's no regclass.

Daniel Sanders via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 26 10:40:03 PST 2018

dsanders added a comment.

>> In https://reviews.llvm.org/D43409#1014897, @dsanders wrote:
>> Hmm, I'm a bit reluctant to make this conditional on whether it's a use/def. Doing that feels like a bigger relaxation on the assertion than should be needed. Is it possible to make it conditional on whether the constraint is specified to be unknown?
> I don't think the unknown part is available in the InstrDescription - we only see the RegClass as -1 for an instruction which uses unknown operands.

I think isReg(), MCOI::OPERAND_UNKNOWN, and regclass -1 combine to indicate unknown but I haven't gone through the whole table to double-check. It doesn't look like there's an accessor to detect MCOI::OPERAND_UNKNOWN in any case.

Given that we can't do better, LGTM with a test case and a nit

In https://reviews.llvm.org/D43409#1019419, @aditya_nandakumar wrote:

> In https://reviews.llvm.org/D43409#1019378, @qcolombet wrote:
> > LGTM.
> >
> > Any chance you could add a test case?
> I don't see any upstream GISel target using unknown as operand type and I'm not sure if creating a fake instruction for a target and then selecting it + constraining it is straightforward/necessary.

AMDGPU uses it a couple times in their tablegen definitions. If we're importing those rules you can probably use that

Comment at: lib/CodeGen/GlobalISel/Utils.cpp:62-63
+    assert((!isTargetSpecificOpcode(II.getOpcode()) || RegMO.isUse()) &&
            "Only target independent instructions are allowed to have operands "
-           "with no register class constraints");
+           "with no register class constraints unless they are uses");
     // FIXME: Just bailing out like this here could be not enough, unless we
It could be clearer that 'unless they are uses' refers to the operands.



More information about the llvm-commits mailing list