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

Bill Seurer seurer at linux.vnet.ibm.com
Mon Oct 6 14:09:33 PDT 2014


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.

> ================
> 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.


> http://reviews.llvm.org/D5362



-- 

-Bill Seurer




More information about the llvm-commits mailing list