[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 15:09:58 PST 2018


dsanders added a comment.

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

> In https://reviews.llvm.org/D43409#1019482, @dsanders wrote:
>
> > >> 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
>
>
> I don't think AMDGPU imports any rules from what I see.


In that case I don't think this is test-able in-tree. LGTM with just the nit


Repository:
  rL LLVM

https://reviews.llvm.org/D43409





More information about the llvm-commits mailing list