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

Bill Schmidt wschmidt at linux.vnet.ibm.com
Mon Sep 22 12:03:28 PDT 2014


On Mon, 2014-09-22 at 13:57 -0500, Hal Finkel wrote:
> ----- Original Message -----
> > From: "Bill Schmidt" <wschmidt at linux.vnet.ibm.com>
> > To: "Eric Christopher" <echristo at gmail.com>
> > Cc: reviews+D5362+public+ecca475fdb0fc511 at reviews.llvm.org, seurer at linux.vnet.ibm.com, "Daniel Sanders"
> > <daniel.sanders at imgtec.com>, "reed kotler" <rkotler at mips.com>, "Hal Finkel" <hfinkel at anl.gov>,
> > llvm-commits at cs.uiuc.edu
> > Sent: Monday, September 22, 2014 1:50:43 PM
> > Subject: Re: [PATCH] [PATCH] Test case and FastISel fixes with FeatureVSX enabled
> > 
> > On Mon, 2014-09-22 at 10:05 -0700, Eric Christopher wrote:
> > > >> 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.
> > > >
> > > 
> > > Not the register classes, but the types that would lead to those
> > > classes. (f128 probably, etc)
> > 
> > Right, I misspoke (with my phalanges).  I think you should be able to
> > just return false when VSX is enabled and VT.isVector() is true.
> 
> And for f64 too. It's that whole 'scalar' part of your 'vector-scalar extensions' ;)

Eh, right. :)  Hey, I've been on vacation, I need a couple days to get
my head back in the game...

Bill

> 
>  -Hal
> 
> > 
> > > 
> > > -eric
> > > 
> > > > 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
> > > >>
> > > >>
> > > >
> > > >
> > > 
> > 
> > 
> > 
> 





More information about the llvm-commits mailing list