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

[Hal Finkel]

----- Original Message -----
> From: "Eric Christopher" <echristo at gmail.com>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: seurer at linux.vnet.ibm.com, "Daniel Sanders" <daniel.sanders at imgtec.com>, "reed kotler" <rkotler at mips.com>, "Bill
> Schmidt" <wschmidt at linux.vnet.ibm.com>, llvm-commits at cs.uiuc.edu,
> reviews+D5362+public+ecca475fdb0fc511 at reviews.llvm.org
> Sent: Friday, September 19, 2014 1:28:02 PM
> Subject: Re: [PATCH] [PATCH] Test case and FastISel fixes with FeatureVSX enabled
> 
> On Fri, Sep 19, 2014 at 11:19 AM, Hal Finkel <hfinkel at anl.gov> wrote:
> > ----- Original Message -----
> >> From: "Eric Christopher" <echristo at gmail.com>
> >> To: reviews+D5362+public+ecca475fdb0fc511 at reviews.llvm.org
> >> Cc: seurer at linux.vnet.ibm.com, "Daniel Sanders"
> >> <daniel.sanders at imgtec.com>, "reed kotler" <rkotler at mips.com>,
> >> "Bill
> >> Schmidt" <wschmidt at linux.vnet.ibm.com>, "Hal Finkel"
> >> <hfinkel at anl.gov>, llvm-commits at cs.uiuc.edu
> >> Sent: Friday, September 19, 2014 11:52:23 AM
> >> Subject: Re: [PATCH] [PATCH] Test case and FastISel fixes with
> >> FeatureVSX enabled
> >>
> >> On Fri, Sep 19, 2014 at 5:17 AM, hfinkel at anl.gov <hfinkel at anl.gov>
> >> 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.
> >> >
> >>
> >> What about builtins? If you don't enable the subtarget feature you
> >> won't be able to select any code that uses VSX in fast isel mode -
> >> even if you could with the selection dag.
> >
> > We don't have any currently ;) -- but this is a good point. Long
> > term, at least, this definitely needs to be fixed properly.
> >
> > Another options is just to refactor the check into a separate
> > function so that we're not repeating the comment explaining what
> > is going on in many places. That's also fine with me.
> >
> 
> Right, that's what I meant by "handled in the isTypeLegal machinery"
> in my original feedback.

Ah, right. That also makes sense. We could return false from isTypeLegal when VSX is enabled for f64, etc.

 -Hal

> 
> -eric
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory