[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