[PATCH] D17850: Power9 - Implement byte comparison and count trailing zero instructions

[Chuang-Yu Cheng via llvm-commits]

cycheng added inline comments.

================
Comment at: lib/Target/PowerPC/PPCInstr64Bit.td:567
@@ -566,1 +566,3 @@
                            IIC_IntCompare>, isPPC64;
+  def CMPRB  : X_BF3_L1_RS5_RS5<31, 192, (outs crbitrc:$BF),
+                                 (ins u1imm:$L, g8rc:$rA, g8rc:$rB),
----------------
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.

================
Comment at: lib/Target/PowerPC/PPCInstr64Bit.td:567
@@ -566,1 +566,3 @@
                            IIC_IntCompare>, isPPC64;
+  def CMPRB  : X_BF3_L1_RS5_RS5<31, 192, (outs crbitrc:$BF),
+                                 (ins u1imm:$L, g8rc:$rA, g8rc:$rB),
----------------
cycheng wrote:
> 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.
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

================
Comment at: lib/Target/PowerPC/PPCInstrFormats.td:767
@@ +766,3 @@
+
+// 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,
----------------
amehsan wrote:
> You can probably remove the comment. Does it help the reader to know this class is very similar to XForm_17?
I think it's helpful : D


Repository:
  rL LLVM

http://reviews.llvm.org/D17850