This patch implement the following instructions:
- addpcis subpcis
- maddhd maddhdu maddld
- modsw moduw modsd modud
- darn
- extswsli extswsli.
- setb
- dtstsfi dtstsfiq
Total 15 instructions
Paths
| Differential D17885
[Power9] Implement add-pc, multiply-add, modulo, extend-sign-shift, random number, set bool, and dfp test significance ClosedPublic Authored by cycheng on Mar 4 2016, 5:44 AM.
Details
Diff Detail Event Timeline
cycheng added inline comments.
Comment Actions I think we probably want to ensure we emit these instructions only when targeting Power 9 processors. So we should probably have some "Requires" in here for things that will be defined on Power9 and later only. Sure it doesn't matter now, but when we start adding SDAG patterns to these, we will be able to emit them on older processors, won't we? cycheng edited edge metadata. cycheng marked 2 inline comments as done. Comment ActionsFixed issues pointed out by nemanjai (http://reviews.llvm.org/D17885#369199)
Comment Actions Agree, something like Requires<[HasP9]>. def FeatureP9 : SubtargetFeature<"power9", "HasP9", "true", "Enable POWER9 instructions", [FeatureP9Vector, Power9SpecificFeatures]>; Comment Actions I agree that it should be rather general like your suggestion of the power9 feature. However, I really think we shouldn't focus this on the chip that implements the ISA but the ISA version itself. That way, other chips that implement the ISA do not have to pretend they're Power 9 chips. However, I don't feel all that strongly about this so I'd like some of the others to comment on this as well (maybe @hfinkel or @kbarton). In review for the atomic instructions (http://reviews.llvm.org/D18032) I implemented what I thought might make sense, but in any case, we should come to a consensus. Comment Actions
I don't think we need to change existing names even though I feel that referencing the chip in the feature name can be misleading. But when the feature is simply called "FeatureP9", I think that's a bit misleading if any other chips end up implementing ISA 3.0. Comment Actions
I agree with @nemanjai. Please use the ISA3.0 feature, from D18032 to guard these instructions.
cycheng edited edge metadata. cycheng marked an inline comment as done. Comment Actions
This revision is now accepted and ready to land.Apr 1 2016, 12:36 PM
Revision Contents
Diff 52327 lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp
lib/Target/PowerPC/Disassembler/PPCDisassembler.cpp
lib/Target/PowerPC/PPCInstr64Bit.td
lib/Target/PowerPC/PPCInstrFormats.td
lib/Target/PowerPC/PPCInstrInfo.td
lib/Target/PowerPC/PPCRegisterInfo.td
lib/Target/PowerPC/README_P9.txt
test/MC/Disassembler/PowerPC/ppc64-encoding.txt
test/MC/PowerPC/ppc64-encoding-ext.s
test/MC/PowerPC/ppc64-encoding.s
|
Please move this definition up to PPCInstrFormats.td since it is used not only in this file, but in PPCInstInfo.td. I realize that everything defined here is available in PPCInstrInfo.td after the include line but nonetheless, I feel that common definitions should go in the common file.