[PATCH] Add LLVM support for remaining integer divide and permute instructions from ISA 2.06

Bill Schmidt wschmidt at linux.vnet.ibm.com
Tue Mar 24 06:35:55 PDT 2015


On Mon, 2015-03-23 at 21:21 +0000, hfinkel at anl.gov wrote:
> REPOSITORY
>   rL LLVM
> 
> ================
> Comment at: lib/Target/PowerPC/PPC.td:163
> @@ +162,3 @@
> +def ProcessorFeatures {
> +  list<SubtargetFeature> Power7FeatureList =
> +      [FeatureISA206, DirectivePwr7, FeatureAltivec, FeatureVSX,
> ----------------
> wschmidt wrote:
> > nemanjai wrote:
> > > hfinkel wrote:
> > > > wschmidt wrote:
> > > > > I don't care for the FeatureISA206 choice.  It contradicts the existing design (there are plenty of features called out that are also in ISA 2.06, so why is this one special?), and is redundant with Power7FeatureList (if you have a Power7FeatureList, shouldn't it just be previous features + FeatureISA206 under this philosophy?).
> > > > > 
> > > > > I think you should go back to Hal's suggestion and provide relevant features that are grouped in a reasonable way.  For this patch, that would be something like FeatureExtDiv and FeatureBitPerm.  I don't think we have a need to disrupt the existing design.
> > > > I agree with Bill agreeing with me ;)
> > > > 
> > > > That having been said, I would like to get a better handle on the features associated with ISA revisions, and group those together. So please add fine-grained features (for one thing, we'll likely have CodeGen support for these at some point, at least bpermd). As a separate change, we can add feature groups for the ISA revisions, and then use those to define the default sets of features for the processors defined.
> > > > 
> > > OK, so to summarize:
> > > - In this patch, add fine-grained control for the instructions that are implemented (I will add FeatureExtDiv and FeatureBitPerm - should I add a "d" at the end of that?)
> > > - In a future patch, provide something along the lines of these feature groups (FeatureISA206 in this case)
> > You can add a "D" if you like; I don't care either way.  It protects against a future feature that does bit permute on another data type, I suppose.
> Yes, that sounds like a good plan. I think adding the D makes sense, I'd just name it, like others, based on the particular instruction: FeatureBPERMD since it refers to the particular instruction.

I still don't really understand what the feature groups buy us.  We
already have -mcpu=pwr7, -mcpu=pwr8 that map to exactly these things.
Why do we need another mechanism?  I feel like I'm missing a fundamental
point somewhere.

Bill

> 
> http://reviews.llvm.org/D8406
> 
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
> 
> 





More information about the llvm-commits mailing list