[PATCH] [PATCH] Test case and FastISel fixes with FeatureVSX enabled

[Bill Schmidt]

On Mon, 2014-09-22 at 11:59 -0500, Bill Schmidt wrote:
> On Mon, 2014-09-22 at 15:11 +0000, Bill Seurer wrote:
> > >>! In D5362#12, @hfinkel wrote:
> > > Thanks for working on this!
> > > 
> > > Regarding approach, instead of disabling parts of FastISel when VSX is enabled, why don't we disable VSX when FastISel would be used? For example, what if in PPCSubtarget::initSubtargetFeatures we added something like this after the call to ParseSubtargetFeatures:
> > > 
> > >   if (OptLevel == CodeGenOpt::None) or even better if possible (TM.Options.EnableFastISel)
> > >     HasVSX = false;
> > > 
> > > (we already do something similar for little-Endian mode currently). Then we just need to add an assert in the FastISel implementation that VSX is not enabled.
> > 
> > How about changing getRegForValue to return 0 if it ends up with a VSX register.  All the uses of getRegForValue already check for a 0 return value.  That moves the code to just one spot and there are already multiple checks there for things that FastISel can't handle.
> 
> Hi Bill,
> 
> I believe this is generally what Eric is suggesting, but
> FastISel::getRegForValue() is in the common code, so we have to change
> it in one of the backend-specific calls.  There is a call to
> TLI.isTypeLegal(VT) in FastISel::getRegForValue() that will call into
> PPCFastISel.isTypeLegal().  So you can add a check there for the VSX
> register classes and return false for now, and that will take care of it
> everywhere.

Also, while you're at it, please remove the FIXME in getRegForValue()
since it will no longer be appropriate to factor this into the shared
base class, and take that out of the TBD list at the top of the module.
It would be good to add verbage about the remaining suggestions (Eric's
(b) thru (d)) into the TBD list.

Thanks!
Bill

> 
> Thanks,
> Bill
> > 
> > > As a general note, please upload full-context diffs in the future (see http://llvm.org/docs/Phabricator.html for instructions).
> > 
> > Will do.
> > 
> > http://reviews.llvm.org/D5362
> > 
> > 
>