Power9 - Implement byte comparison and count trailing zero instructions

[nemanjai]

Summary

This patch implements the following instructions:
cmprb
cmpeqb
cnttzw
cnttzw.
cnttzd
cnttzd.

It also implements a list of P9 specific target features that we will add to when implementing new features so that we don't forget any.
Finally, the patterns for the count trailing zeros are provided because they're obvious.

[nemanjai]

I forgot to add comments about this to README_P9.txt.

[amehsan]

just a few minor nits. I did not review the tests though. I am going to learn that part of the code today :)

lib/Target/PowerPC/PPC.td

154–159

It will make the code more readable to have something in the name or description of the feature that indicates the feature was introduced in P9. I know we do not have it for many features, but as features are added, some extra clarity will be helpful.

lib/Target/PowerPC/PPCInstr64Bit.td

570

any reason for different indentation of 'Requires' clause? I noticed that sometimes we use this different indentation for 'Requires' and sometimes not. Couldn't figure out justification for having inconsistent indentation.

592

same question here.

626

I think you introduced an extra space unintentionally.

lib/Target/PowerPC/PPCInstrFormats.td

767

You can probably remove the comment. Does it help the reader to know this class is very similar to XForm_17?

770

different indentation in here and the other class that you defined right before this.

[cycheng]

lib/Target/PowerPC/PPCInstr64Bit.td

567

These instructions are enclosed in "let PPC970_Unit = 1 in { ... }"

It looks like PPC970 indicating specific processor, i.e. G5, and it's purpose seems for PPCHazardRecognizer970 instruction scheduling implementation, so I don't know whether "let PPC970_Unit ..." is required for Power9 instructions or not.

567

Should we move CMPRB to PPCInstrInfo.td, and provide CMPRB8 definition here? Because it looks like 32-bit instructions:

(RA)56:63
(RB)32:63

lib/Target/PowerPC/PPCInstrFormats.td

767

I think it's helpful : D

[nemanjai]

lib/Target/PowerPC/PPC.td

154–159

I think we generally did not mention any of the processors a feature is implemented on because such features can become part of multiple chips. For example, FeatureLDBRX is available on Power7, but it's also available on the A2 chip.

That being said, I'm not terribly opposed to having P9 in the name, but I think I'll defer the final decision to @hfinkel.

lib/Target/PowerPC/PPCInstr64Bit.td

567

This is a good point. I initially implemented it that way and then removed it from the 32-bit definition. However, both the Hi and Lo range delimiters are in the lower 32-bits of the registers so this should be perfectly usable in 32-bit mode.
I'll add it there.

567

I have added them to this section assuming it is sufficiently similar to these instructions from the standpoint of scheduling. However, I don't understand the details of how this information is used. Hopefully @hfinkel can provide some guidance here.

570

I realize that there are some places where the "Requires" base class appears on the start of a line and it is aligned with the name of the instruction. However, since the definition of the instruction inherits from two classes (namely, X_BF3_L1_RS5_RS5 and Requires in this case), I feel that both classes inherited should be aligned after the colon.

626

The intention was to align everything to the right of "58" to the newly added instruction (similarly to CNTLZW in the 32-bit instruction definition file). I know it's a rather unrelated change, I just thought it looked nicer all ligned up :).

lib/Target/PowerPC/PPCInstrFormats.td

767

Me too. I think it's a quick reference for someone that is looking for a form to use and XForm_17 is almost right. Also, if we ever get around to renaming forms to have a common scheme, it can be helpful.

770

That's a good point. I don't know how this one got out. I'll fix it. Thanks.

[kbarton]

lib/Target/PowerPC/PPC.td

160

I don't see the need for adding a subtarget feature for these instructions. Why is this necessary?

[nemanjai]

lib/Target/PowerPC/PPC.td

160

It is not necessary to add individual features for these instructions. However, it doesn't seem right to guard these by the only existing Power9 features (namely Power9Vector, Power9Altivec). Also, adding a single feature for all Power9 scalar instructions seems to break with tradition.
Could you please suggest an alternative to defining these two features?

[kbarton]

Please replace the FeatureCharCmp and FeatureCTTZ features with the more generic ISA3.0 feature (or something similar).
With this change, this LGTM.

lib/Target/PowerPC/PPC.td

160

Based on discussions today we agreed to add a generic feature to indicate ISA3.0. The name should reflect the actual ISA version number, and not be tied to any specific chip implementation (i.e., P9). The feature definitions for P9 (e.g., P9-vector and P9-altivec) should prereq this ISA3.0 feature. Similarly, the definition of the P9 sub target should include this as well.

[nemanjai]

Committed revision 266228.

lib/Target/PowerPC/PPC.td

 def FeatureMFTB   : SubtargetFeature<"", "FeatureMFTB", "true",
                                         "Implement mftb using the mfspr instruction">;
 def FeatureFusion : SubtargetFeature<"fusion", "HasFusion", "true",
                                      "Target supports add/load integer fusion.">;
 def FeatureFloat128 :
   SubtargetFeature<"float128", "HasFloat128", "true",
                    "Enable the __float128 data type for IEEE-754R Binary128.",
                    [FeatureVSX]>;
+def FeatureCharCmp   :
+  SubtargetFeature<"char-cmp", "HasCharCmp", "true",
+                   "Enable the Character-Type compare instructions">;
+def FeatureCTTZ   :
+  SubtargetFeature<"cttz", "HasCTTZ", "true",
+                   "Enable count trailing zeros instructions">;
 
 def DeprecatedDST    : SubtargetFeature<"", "DeprecatedDST", "true",
   "Treat vector data stream cache control instructions as deprecated">;
 
 /*  Since new processors generally contain a superset of features of those that
     came before them, the idea is to make implementations of new processors
     less error prone and easier to read.
     Namely:
         list<SubtargetFeature> Power8FeatureList = ...
list<SubtargetFeature> Power7FeatureList =
        FeatureBPERMD, FeatureExtDiv,
        FeatureMFTB, DeprecatedDST];
   list<SubtargetFeature> Power8SpecificFeatures =
       [DirectivePwr8, FeatureP8Altivec, FeatureP8Vector, FeatureP8Crypto,
        FeatureHTM, FeatureDirectMove, FeatureICBT, FeaturePartwordAtomic,
        FeatureFusion];
   list<SubtargetFeature> Power8FeatureList =
       !listconcat(Power7FeatureList, Power8SpecificFeatures);
+  list<SubtargetFeature> Power9SpecificFeatures =
+      [FeatureP9Altivec, FeatureP9Vector, FeatureCharCmp, FeatureCTTZ];
 }
 
 // Note: Future features to add when support is extended to more
 // recent ISA levels:
 //
 // DFP          p6, p6x, p7        decimal floating-point instructions
 // POPCNTB      p5 through p7      popcntb and related instructions
 

lib/Target/PowerPC/PPCInstr64Bit.td

def CMPD   : XForm_16_ext<31, 0, (outs crrc:$crD), (ins g8rc:$rA, g8rc:$rB),
                             "cmpd $crD, $rA, $rB", IIC_IntCompare>, isPPC64;
   def CMPLD  : XForm_16_ext<31, 32, (outs crrc:$crD), (ins g8rc:$rA, g8rc:$rB),
                             "cmpld $crD, $rA, $rB", IIC_IntCompare>, isPPC64;
   def CMPDI  : DForm_5_ext<11, (outs crrc:$crD), (ins g8rc:$rA, s16imm64:$imm),
                            "cmpdi $crD, $rA, $imm", IIC_IntCompare>, isPPC64;
   def CMPLDI : DForm_6_ext<10, (outs crrc:$dst), (ins g8rc:$src1, u16imm64:$src2),
                            "cmpldi $dst, $src1, $src2",
                            IIC_IntCompare>, isPPC64;
+  def CMPRB  : X_BF3_L1_RS5_RS5<31, 192, (outs crbitrc:$BF),
+                                 (ins u1imm:$L, g8rc:$rA, g8rc:$rB),
+                                 "cmprb $BF, $L, $rA, $rB", IIC_IntCompare>,
+                Requires<[HasCharCmp]>;
+  def CMPEQB : X_BF3_RS5_RS5<31, 224, (outs crbitrc:$BF),
+                             (ins g8rc:$rA, g8rc:$rB), "cmpeqb $BF, $rA, $rB",
+                             IIC_IntCompare>, Requires<[HasCharCmp]>;
 }
 
 let hasSideEffects = 0 in {
 defm SLD  : XForm_6r<31,  27, (outs g8rc:$rA), (ins g8rc:$rS, gprc:$rB),
                      "sld", "$rA, $rS, $rB", IIC_IntRotateD,
                      [(set i64:$rA, (PPCshl i64:$rS, i32:$rB))]>, isPPC64;
 defm SRD  : XForm_6r<31, 539, (outs g8rc:$rA), (ins g8rc:$rS, gprc:$rB),
                      "srd", "$rA, $rS, $rB", IIC_IntRotateD,
                      [(set i64:$rA, (PPCsrl i64:$rS, i32:$rB))]>, isPPC64;
 defm SRAD : XForm_6rc<31, 794, (outs g8rc:$rA), (ins g8rc:$rS, gprc:$rB),
                       "srad", "$rA, $rS, $rB", IIC_IntRotateD,
                       [(set i64:$rA, (PPCsra i64:$rS, i32:$rB))]>, isPPC64;
 
 let Interpretation64Bit = 1, isCodeGenOnly = 1 in {
 defm CNTLZW8 : XForm_11r<31,  26, (outs g8rc:$rA), (ins g8rc:$rS),
                         "cntlzw", "$rA, $rS", IIC_IntGeneral, []>;
+defm CNTTZW8 : XForm_11r<31, 538, (outs g8rc:$rA), (ins g8rc:$rS),
+                        "cnttzw", "$rA, $rS", IIC_IntGeneral, []>,
+               Requires<[HasCTTZ]>;
 
 defm EXTSB8 : XForm_11r<31, 954, (outs g8rc:$rA), (ins g8rc:$rS),
                         "extsb", "$rA, $rS", IIC_IntSimple,
                         [(set i64:$rA, (sext_inreg i64:$rS, i8))]>;
 defm EXTSH8 : XForm_11r<31, 922, (outs g8rc:$rA), (ins g8rc:$rS),
                         "extsh", "$rA, $rS", IIC_IntSimple,
                         [(set i64:$rA, (sext_inreg i64:$rS, i16))]>;
 

 let Interpretation64Bit = 1, isCodeGenOnly = 1 in
 defm EXTSW_32_64 : XForm_11r<31, 986, (outs g8rc:$rA), (ins gprc:$rS),
                              "extsw", "$rA, $rS", IIC_IntSimple,
                              [(set i64:$rA, (sext i32:$rS))]>, isPPC64;
 
 defm SRADI  : XSForm_1rc<31, 413, (outs g8rc:$rA), (ins g8rc:$rS, u6imm:$SH),
                          "sradi", "$rA, $rS, $SH", IIC_IntRotateDI,
                          [(set i64:$rA, (sra i64:$rS, (i32 imm:$SH)))]>, isPPC64;
-defm CNTLZD : XForm_11r<31, 58, (outs g8rc:$rA), (ins g8rc:$rS),
+defm CNTLZD : XForm_11r<31,  58, (outs g8rc:$rA), (ins g8rc:$rS),
                         "cntlzd", "$rA, $rS", IIC_IntGeneral,
                         [(set i64:$rA, (ctlz i64:$rS))]>;
+defm CNTTZD : XForm_11r<31, 570, (outs g8rc:$rA), (ins g8rc:$rS),
+                        "cnttzd", "$rA, $rS", IIC_IntGeneral,
+                        [(set i64:$rA, (cttz i64:$rS))]>, Requires<[HasCTTZ]>;
 def POPCNTD : XForm_11<31, 506, (outs g8rc:$rA), (ins g8rc:$rS),
                        "popcntd $rA, $rS", IIC_IntGeneral,
                        [(set i64:$rA, (ctpop i64:$rS))]>;
 def BPERMD : XForm_6<31, 252, (outs g8rc:$rA), (ins g8rc:$rS, g8rc:$rB),
                      "bpermd $rA, $rS, $rB", IIC_IntGeneral,
                      [(set i64:$rA, (int_ppc_bpermd g8rc:$rS, g8rc:$rB))]>,
                      isPPC64, Requires<[HasBPERMD]>;
 

lib/Target/PowerPC/PPCInstrFormats.td

class XForm_htm3<bits<6> opcode, bits<10> xo, dag OOL, dag IOL, string asmstr,
   bit RC = 0;
 
   let Inst{6-8}   = BF;
   let Inst{9-20}  = 0;
   let Inst{21-30} = xo;
   let Inst{31}    = RC;
 }
 
+class X_BF3_L1_RS5_RS5<bits<6> opcode, bits<10> xo, dag OOL, dag IOL,
+                       string asmstr, InstrItinClass itin>
+  : I<opcode, OOL, IOL, asmstr, itin> {
+  bits<3> BF;
+  bits<1> L;
+  bits<5> RA;
+  bits<5> RB;
+
+  let Inst{6-8}   = BF;
+  let Inst{9}     = 0;
+  let Inst{10}    = L;
+  let Inst{11-15} = RA;
+  let Inst{16-20} = RB;
+  let Inst{21-30} = xo;
+  let Inst{31}    = 0;
+}
+
+// Same as XForm_17 but with GPR's and new naming convention
+class X_BF3_RS5_RS5<bits<6> opcode, bits<10> xo, dag OOL, dag IOL,
+                    string asmstr, InstrItinClass itin>
+         : I<opcode, OOL, IOL, asmstr, itin> {
+  bits<3> BF;
+  bits<5> RA;
+  bits<5> RB;
+
+  let Inst{6-8}   = BF;
+  let Inst{9-10}  = 0;
+  let Inst{11-15} = RA;
+  let Inst{16-20} = RB;
+  let Inst{21-30} = xo;
+  let Inst{31}    = 0;
+}
+
 // e.g. [PO VRT XO VRB XO /] or [PO VRT XO VRB XO RO]
 class X_RD5_XO5_RS5<bits<6> opcode, bits<5> xo2, bits<10> xo, dag OOL, dag IOL,
                     string asmstr, InstrItinClass itin, list<dag> pattern>
   : XForm_base_r3xo<opcode, xo, OOL, IOL, asmstr, itin, pattern> {
   let A = xo2;
 }
 
 // XX*-Form (VSX)

lib/Target/PowerPC/PPCInstrInfo.td

 def IsE500  : Predicate<"PPCSubTarget->isE500()">;
 def HasSPE  : Predicate<"PPCSubTarget->HasSPE()">;
 def HasICBT : Predicate<"PPCSubTarget->hasICBT()">;
 def HasPartwordAtomics : Predicate<"PPCSubTarget->hasPartwordAtomics()">;
 def NoNaNsFPMath : Predicate<"TM.Options.NoNaNsFPMath">;
 def NaNsFPMath   : Predicate<"!TM.Options.NoNaNsFPMath">;
 def HasBPERMD : Predicate<"PPCSubTarget->hasBPERMD()">;
 def HasExtDiv : Predicate<"PPCSubTarget->hasExtDiv()">;
+def HasCharCmp : Predicate<"PPCSubTarget->hasCharCmp()">;
+def HasCTTZ : Predicate<"PPCSubTarget->hasCTTZ()">;
 
 //===----------------------------------------------------------------------===//
 // PowerPC Multiclass Definitions.
 
 multiclass XForm_6r<bits<6> opcode, bits<10> xo, dag OOL, dag IOL,
                     string asmbase, string asmstr, InstrItinClass itin,
                     list<dag> pattern> {
   let BaseName = asmbase in {

 let PPC970_Unit = 1 in {  // FXU Operations.
 let hasSideEffects = 0 in {
 defm SRAWI : XForm_10rc<31, 824, (outs gprc:$rA), (ins gprc:$rS, u5imm:$SH),
                         "srawi", "$rA, $rS, $SH", IIC_IntShift,
                         [(set i32:$rA, (sra i32:$rS, (i32 imm:$SH)))]>;
 defm CNTLZW : XForm_11r<31,  26, (outs gprc:$rA), (ins gprc:$rS),
                         "cntlzw", "$rA, $rS", IIC_IntGeneral,
                         [(set i32:$rA, (ctlz i32:$rS))]>;
+defm CNTTZW : XForm_11r<31, 538, (outs gprc:$rA), (ins gprc:$rS),
+                        "cnttzw", "$rA, $rS", IIC_IntGeneral,
+                        [(set i32:$rA, (cttz i32:$rS))]>, Requires<[HasCTTZ]>;
 defm EXTSB  : XForm_11r<31, 954, (outs gprc:$rA), (ins gprc:$rS),
                         "extsb", "$rA, $rS", IIC_IntSimple,
                         [(set i32:$rA, (sext_inreg i32:$rS, i8))]>;
 defm EXTSH  : XForm_11r<31, 922, (outs gprc:$rA), (ins gprc:$rS),
                         "extsh", "$rA, $rS", IIC_IntSimple,
                         [(set i32:$rA, (sext_inreg i32:$rS, i16))]>;
 
 let isCommutable = 1 in

lib/Target/PowerPC/PPCSubtarget.h

protected:
   bool IsLittleEndian;
   bool HasICBT;
   bool HasInvariantFunctionDescriptors;
   bool HasPartwordAtomics;
   bool HasDirectMove;
   bool HasHTM;
   bool HasFusion;
   bool HasFloat128;
+  bool HasCharCmp;
+  bool HasCTTZ;
 
   /// When targeting QPX running a stock PPC64 Linux kernel where the stack
   /// alignment has not been changed, we need to keep the 16-byte alignment
   /// of the stack.
   bool IsQPXStackUnaligned;
 
   const PPCTargetMachine &TM;
   PPCFrameLowering FrameLowering;
unsigned getPlatformStackAlignment() const {
     if ((hasQPX() || isBGQ()) && !isQPXStackUnaligned())
       return 32;
 
     return 16;
   }
   bool hasHTM() const { return HasHTM; }
   bool hasFusion() const { return HasFusion; }
   bool hasFloat128() const { return HasFloat128; }
+  bool hasCharCmp() const { return HasCharCmp; }
+  bool hasCTTZ() const { return HasCTTZ; }
 
   const Triple &getTargetTriple() const { return TargetTriple; }
 
   /// isDarwin - True if this is any darwin platform.
   bool isDarwin() const { return TargetTriple.isMacOSX(); }
   /// isBGQ - True if this is a BG/Q platform.
   bool isBGQ() const { return TargetTriple.getVendor() == Triple::BGQ; }
 

lib/Target/PowerPC/PPCSubtarget.cpp

void PPCSubtarget::initializeEnvironment() {
   HasICBT = false;
   HasInvariantFunctionDescriptors = false;
   HasPartwordAtomics = false;
   HasDirectMove = false;
   IsQPXStackUnaligned = false;
   HasHTM = false;
   HasFusion = false;
   HasFloat128 = false;
+  HasCharCmp = false;
+  HasCTTZ = false;
 }
 
 void PPCSubtarget::initSubtargetFeatures(StringRef CPU, StringRef FS) {
   // Determine default and user specified characteristics
   std::string CPUName = CPU;
   if (CPUName.empty() || CPU == "generic") {
     // If cross-compiling with -march=ppc64le without -mcpu
     if (TargetTriple.getArch() == Triple::ppc64le)

lib/Target/PowerPC/README_P9.txt

- Round to Quad-Precision Integer: xsrqpi xsrqpix
   . Provide builtin?
     (set f128:$vT, (int_ppc_vsx_xsrqpi f128:$vB))
     (set f128:$vT, (int_ppc_vsx_xsrqpix f128:$vB))
 
 - Round Quad-Precision to Double-Extended Precision (fp80): xsrqpxp
   . Provide builtin?
     (set f128:$vT, (int_ppc_vsx_xsrqpxp f128:$vB))
 
+Fixed Point Facility:
+
+- Exploit cmprb and cmpeqb (perhaps for something like
+  isalpha/isdigit/isupper/islower and isspace respectivelly). This can
+  perhaps be done through a builtin.
+
+- Provide testing for cnttz[dw]

test/MC/Disassembler/PowerPC/ppc64-encoding.txt

 0x7d 0x03 0x20 0x00
 
 # CHECK: cmplwi 2, 3, 128                
 0x29 0x03 0x00 0x80
 
 # CHECK: cmplw 2, 3, 4                   
 0x7d 0x03 0x20 0x40
 
+# CHECK: cmprb 3, 1, 7, 17
+0x7d 0xa7 0x89 0x80
+
+# CHECK: cmpeqb 3, 7, 17
+0x7d 0x87 0x89 0xc0
+
+# CHECK: cnttzw 7, 17
+0x7e 0x27 0x04 0x34
+
+# CHECK: cnttzw. 7, 17
+0x7e 0x27 0x04 0x35
+
+# CHECK: cnttzd 7, 17
+0x7e 0x27 0x04 0x74
+
+# CHECK: cnttzd. 7, 17
+0x7e 0x27 0x04 0x75
+
 # CHECK: twllti 3, 4
 0x0c 0x43 0x00 0x04
 
 # CHECK: twllt 3, 4
 0x7c 0x43 0x20 0x08
 
 # CHECK: tdllti 3, 4
 0x08 0x43 0x00 0x04

test/MC/PowerPC/ppc64-encoding.s

 # CHECK-LE: cmpw 2, 3, 4                    # encoding: [0x00,0x20,0x03,0x7d]
             cmp 2, 0, 3, 4
 # CHECK-BE: cmplwi 2, 3, 128                # encoding: [0x29,0x03,0x00,0x80]
 # CHECK-LE: cmplwi 2, 3, 128                # encoding: [0x80,0x00,0x03,0x29]
             cmpli 2, 0, 3, 128
 # CHECK-BE: cmplw 2, 3, 4                   # encoding: [0x7d,0x03,0x20,0x40]
 # CHECK-LE: cmplw 2, 3, 4                   # encoding: [0x40,0x20,0x03,0x7d]
             cmpl 2, 0, 3, 4
+# CHECK-BE: cmprb 3, 1, 7, 17               # encoding: [0x7d,0xa7,0x89,0x80]
+# CHECK-LE: cmprb 3, 1, 7, 17               # encoding: [0x80,0x89,0xa7,0x7d]
+            cmprb 3, 1, 7, 17
+# CHECK-BE: cmpeqb 3, 7, 17                 # encoding: [0x7d,0x87,0x89,0xc0]
+# CHECK-LE: cmpeqb 3, 7, 17                 # encoding: [0xc0,0x89,0x87,0x7d]
+            cmpeqb 3, 7, 17
+# CHECK-BE: cnttzw 7, 17                    # encoding: [0x7e,0x27,0x04,0x34]
+# CHECK-LE: cnttzw 7, 17                    # encoding: [0x34,0x04,0x27,0x7e]
+            cnttzw 7, 17
+# CHECK-BE: cnttzw. 7, 17                   # encoding: [0x7e,0x27,0x04,0x35]
+# CHECK-LE: cnttzw. 7, 17                   # encoding: [0x35,0x04,0x27,0x7e]
+            cnttzw. 7, 17
+# CHECK-BE: cnttzd 7, 17                    # encoding: [0x7e,0x27,0x04,0x74]
+# CHECK-LE: cnttzd 7, 17                    # encoding: [0x74,0x04,0x27,0x7e]
+            cnttzd 7, 17
+# CHECK-BE: cnttzd. 7, 17                   # encoding: [0x7e,0x27,0x04,0x75]
+# CHECK-LE: cnttzd. 7, 17                   # encoding: [0x75,0x04,0x27,0x7e]
+            cnttzd. 7, 17
 
 # Fixed-point trap instructions
 
 # CHECK-BE: twllti 3, 4                     # encoding: [0x0c,0x43,0x00,0x04]
 # CHECK-LE: twllti 3, 4                     # encoding: [0x04,0x00,0x43,0x0c]
             twi 2, 3, 4
 # CHECK-BE: twllt 3, 4                      # encoding: [0x7c,0x43,0x20,0x08]
 # CHECK-LE: twllt 3, 4                      # encoding: [0x08,0x20,0x43,0x7c]