[LLVMdev] Types vs. register classes in instruction patterns -- effect on FastISel

[Bill Schmidt]

On Fri, 2013-05-17 at 15:32 -0700, Jakob Stoklund Olesen wrote:
> On May 17, 2013, at 2:04 PM, Bill Schmidt <wschmidt at linux.vnet.ibm.com> wrote:
> 
> > Hello,
> > 
> > In http://llvm.org/viewvc/llvm-project?view=revision&revision=177889 and
> > http://llvm.org/viewvc/llvm-project?view=revision&revision=177890 (along
> > with some follow-up patches) the PowerPC back end was changed to use
> > types instead of register classes in instruction patterns.  This matched
> > similar changes that Jakob made for Sparc in r177835.
> > 
> > I've recently come across an unpleasant side-effect of this change.  I
> > started looking at implementing FastISel for PPC64, and discovered that
> > practically all of our patterns were rejected when looking for simple
> > patterns eligible for FastEmit_*.  Stepping through the tablegen code
> > showed that we were rejecting them precisely because the patterns now
> > contain value types instead of register classes (FastISelEmitter.cpp,
> > lines 253ff.):
> > 
> >      // For now, the only other thing we accept is register operands.
> >      const CodeGenRegisterClass *RC = 0;
> >      if (OpLeafRec->isSubClassOf("RegisterOperand"))
> >        OpLeafRec = OpLeafRec->getValueAsDef("RegClass");
> >      if (OpLeafRec->isSubClassOf("RegisterClass"))
> >        RC = &Target.getRegisterClass(OpLeafRec);
> >      else if (OpLeafRec->isSubClassOf("Register"))
> >        RC = Target.getRegBank().getRegClassForRegister(OpLeafRec);
> >      else
> >        return false;
> > 
> > What's the proper thing to do here?  If we can map a ValueType to a
> > register class, then we'd be ok.  I don't immediately see an interface
> > that will do this (indeed, I guess I'm not certain that's guaranteed to
> > be a 1-1 mapping).
> > 
> > This wasn't noticed previously on Sparc or PowerPC since neither has had
> > a FastISel implementation yet.  Thoughts?
> 
> That’s a lot of ‘for nows’ in one function...
> 
> It seems to me that the code is looking at the wrong pattern to find register classes. The input patterns match on types, the instructions in the output patterns specify register classes.
> 
> However, the register class isn’t really used for anything, so maybe the code should simply be expanded to also accept patterns where all the input operands have the same type.

Well, the specific register class isn't used, but the fact that
something register-like was found is used downstream.  For grins I tried
eliminating the code to check for register-ness and found that both the
X86 and ARM targets failed to build.  The .inc file was built for X86,
but the generated code was nonsensical for certain kinds of patterns
(function names containing .getPtrType() as a substring, etc.).  ARM
couldn't build the .inc file at all due to duplicate patterns.

So I think we have to specifically recognize ValueTypes.  To keep things
simple, we could pass in the destination register class from
collectPatterns, which has the output register class of the destination
pattern, and only permit a ValueType representable by that RC.

For an experiment, I tried allowing all ValueType source operands.  This
had no effect on code generated for the ARM and X86 targets, so that's
good.  However, I ran into problems with duplicates generated into the
FastISel tables for PowerPC, due to the multiclass definitions for
recording forms.  So there's still work to be done there.

Thanks,
Bill

> 
> /jakob
>