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

[Hal Finkel]

----- Original Message -----
> From: "Bill Seurer" <seurer at linux.vnet.ibm.com>
> To: reviews+D5362+public+ecca475fdb0fc511 at reviews.llvm.org, "daniel sanders" <daniel.sanders at imgtec.com>,
> rkotler at mips.com, hfinkel at anl.gov, echristo at gmail.com, wschmidt at linux.vnet.ibm.com
> Cc: llvm-commits at cs.uiuc.edu
> Sent: Monday, October 6, 2014 4:09:33 PM
> Subject: Re: [PATCH] [PATCH] Test case and FastISel fixes with FeatureVSX enabled
> 
> On 10/6/2014 4:01 PM, hfinkel at anl.gov wrote:
> > ================
> > Comment at:
> > /home/seurer/llvm/llvm-test/include/llvm/CodeGen/FastISel.h:486
> > @@ +485,3 @@
> > +  virtual bool isTypeLegal(EVT Evt) {
> > +    return TLI.isTypeLegal(Evt);
> > +  }
> > ----------------
> > Do other targets break if you make this call their two-argument
> > isTypeLegal with a throw-away VT argument?
> >
> > I'd rather do this in a way that targets don't need to provide two
> > very-similar overrides and keep them in sync if possible.
> 
> Not all targets have their own isTypeLegal in FastISel.  And x86 has
> a
> three parm version instead of the two parm one.

Ah, okay, there is only one FastISel isTypeLegal overridable function. This solution seems correct and fairly clean. LGTM.

> 
> > ================
> > Comment at:
> > /home/seurer/llvm/llvm-test/lib/Target/PowerPC/PPCFastISel.cpp:264
> > @@ +263,3 @@
> > +  // FIXME: remove when VSX support is added.
> > +  // note: f128 and ppcf128 are not included for now but should be
> > added
> > +  if (PPCSubTarget->hasVSX() && (VT.isVector() || VT == MVT::f64))
> > {
> > ----------------
> > Not sure about f128 (it is not a legal type, and I assume the base
> > class will take care of this), but if ppcf128 should be here,
> > please add it.
> 
> Those came up in discussions but in practice shouldn't appear there
> now.
>   I will remove the comment.

Thanks.

 -Hal

> 
> 
> > http://reviews.llvm.org/D5362
> 
> 
> 
> --
> 
> -Bill Seurer
> 
> 

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