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

[Eric Christopher]

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.

-eric