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

Bill Schmidt wschmidt at linux.vnet.ibm.com
Wed Mar 18 13:39:27 PDT 2015


Hi Nemanja,

Just a few minor inline comments.  Good-looking patch otherwise!


REPOSITORY
  rL LLVM

================
Comment at: include/llvm/IR/IntrinsicsPowerPC.td:50
@@ +49,3 @@
+                                 [IntrNoMem]>;
+  def int_ppc_bpermd : Intrinsic<[llvm_i64_ty], [llvm_i64_ty, llvm_i64_ty],
+                                 [IntrNoMem]>;
----------------
OK, this is a real nit, but -- perhaps add a blank line before bpermd since this one is not a divide-extend as the comment would seem to imply.

================
Comment at: lib/Target/PowerPC/PPCInstr64Bit.td:630
@@ +629,3 @@
+                      "divde. $rT, $rA, $rB", IIC_IntDivD,
+                      []>, isDOT, PPC970_DGroup_Cracked, isPPC64,
+                      Requires<[IsPwr7Up]>;
----------------
As discussed offline, I am bothered that these are not marked first-in-group but the regular divides are.  Please check with Pat Haugen on correctness.

Long term, it makes no sense to have a single set of flags like this for each instruction, since these characteristics can and do change from one implementation to the next.  But that will be a different patch...

================
Comment at: lib/Target/PowerPC/PPCInstrInfo.td:717
@@ -716,1 +716,3 @@
 def NaNsFPMath   : Predicate<"!TM.Options.NoNaNsFPMath">;
+def IsPwr7Up : Predicate<"PPCSubTarget->isPwr7Up()">;
+def IsPwr8Up : Predicate<"PPCSubTarget->isPwr8Up()">;
----------------
I'm a tiny bit not enthused about the name -- IsPwr7OrHigher, perhaps?  Pwr7Up sounds like an energy drink. ;)  If nobody else dislikes it, leave it as is, though.

================
Comment at: lib/Target/PowerPC/README.txt:626
@@ +625,3 @@
+//===-------------------------------------------------------------------------===
+Naming convention for instruction formats is very hap-hazard.
+We have agreed on a naming scheme as follows:
----------------
Nit:  No hyphen in "haphazard."

http://reviews.llvm.org/D8406

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






More information about the llvm-commits mailing list