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

[Bill Seurer]

On 9/25/2014 10:33 AM, Hal Finkel wrote:
> ----- Original Message -----
>> From: "Bill Seurer" <seurer at linux.vnet.ibm.com>
>> To: "Bill Schmidt" <wschmidt at linux.vnet.ibm.com>, "Hal Finkel" <hfinkel at anl.gov>
>> Cc: reviews+D5362+public+ecca475fdb0fc511 at reviews.llvm.org, "Daniel Sanders" <daniel.sanders at imgtec.com>, "reed
>> kotler" <rkotler at mips.com>, llvm-commits at cs.uiuc.edu, "Eric Christopher" <echristo at gmail.com>
>> Sent: Thursday, September 25, 2014 10:21:11 AM
>> Subject: Re: [PATCH] [PATCH] Test case and FastISel fixes with FeatureVSX enabled
>>
>>
>> There is a problem in just trying to use PPCFastISel.isTypeLegal().
>> There are two places in FastISel::getRegForValue where a register is
>> actually gotten. The first place involves the above mentioned call to
>> TLI.isTypeLegal(VT) BUT this does not get to PPCFastISel.isTypeLegal
>> because it goes into TargetLoweringBase::isTypeLegal instead.
>
> Okay, so generally speaking, there are about 8 places in lib/CodeGen/SelectionDAG/FastISel.cpp that call TLI.isTypeLegal (and I see no calls to the FastISel-specific isTypeLegal). Perhaps this should be changed?
>
>   -Hal

I ran into a wrinkle with using the FastISel-specific isTypeLegal.  Its 
second parameter (an MVT) is a reference and it actually updates the 
value of the passed in argument via the reference.  This is different 
behavior than what TLI.isLegal does and most likely not wanted.

While experimenting with this to see what happened anyway I ran into 
something else:

bool SelectionDAGISel::runOnMachineFunction(MachineFunction &mf) {
   // Do some sanity-checking on the command-line options.
   assert((!EnableFastISelVerbose || TM.Options.EnableFastISel) &&
          "-fast-isel-verbose requires -fast-isel");
   assert((!EnableFastISelAbort || TM.Options.EnableFastISel) &&
          "-fast-isel-abort requires -fast-isel");

The second assert states that -fast-isel-abort requires -fast-isel but 
there are many PPC (and maybe other target) test cases that use only 
-fast-isel-abort as an option.  Is this really a requirement?  If so, 
why doesn't -fast-isel-abort either check that -fast-isel was specified 
or even just set it itself?
-- 

-Bill Seurer