[PATCH] D23155: Power9 - Part-word VSX integer scalar loads/stores and sign extend instructions

Kit Barton via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 31 12:58:00 PDT 2016


kbarton added inline comments.

================
Comment at: lib/Target/PowerPC/InstPrinter/PPCInstPrinter.cpp:450
@@ +449,3 @@
+    // names are "v0-v31", so we need to map "v0-v31" to "vs32-vs63"
+    // (Please synchronize with PPCAsmPrinter::printOperand)
+    if ((MII.get(MI->getOpcode()).TSFlags & PPCII::UseVSXReg)) {
----------------
What does this comment mean?
Does this need to be done? If not, can it be removed?

================
Comment at: lib/Target/PowerPC/MCTargetDesc/PPCMCCodeEmitter.cpp:354
@@ -352,1 +353,3 @@
 
+static bool isVRRegister(unsigned Reg) {
+  return Reg >= PPC::V0 && Reg <= PPC::V31;
----------------
This routine is defined as static in at least two files. 
Can we add it to a more general location and just define it once?

================
Comment at: lib/Target/PowerPC/PPCAsmPrinter.cpp:165
@@ -164,1 +164,3 @@
 
+static bool isVFRegister(unsigned Reg) {
+  return Reg >= PPC::VF0 && Reg <= PPC::VF31;
----------------
Same comment as above. Can these go into a more general location?

================
Comment at: lib/Target/PowerPC/PPCAsmPrinter.cpp:185
@@ +184,3 @@
+    // names are "v0-v31", so we need to map "v0-v31" to "vs32-vs63"
+    // (Please synchronize with PPCInstPrinter::printOperand)
+    if (MI->getDesc().TSFlags & PPCII::UseVSXReg) {
----------------
Extraneous comment?

================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:10300
@@ -10317,1 +10299,3 @@
 
+  SDValue FirstOperand(Op.getOperand(0));
+  bool SubWordLoad = FirstOperand.getOpcode() == ISD::LOAD &&
----------------
Was this new section formatted with clang-format?
The indentation looks strange to me, but this might be what it does when you have these types of long lines.


Repository:
  rL LLVM

https://reviews.llvm.org/D23155





More information about the llvm-commits mailing list