[llvm] r358122 - [X86AsmPrinter] refactor to limit use of Modifier. NFC

Nick Desaulniers via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 10 12:01:45 PDT 2019


Author: nickdesaulniers
Date: Wed Apr 10 12:01:44 2019
New Revision: 358122

URL: http://llvm.org/viewvc/llvm-project?rev=358122&view=rev
Log:
[X86AsmPrinter] refactor to limit use of Modifier. NFC

Summary:
The Modifier memory operands is used in 2 cases of memory references
(H & P ExtraCodes). Rather than pass around the likely nullptr Modifier,
refactor the handling of the Modifier out from printOperand().

The refactorings in this patch:
- Don't forward declare printOperand, move its definition up.
  - The diff makes it look like there's a change to printPCRelImm
    (narrator: there's not).
- Create printModifiedOperand()
  - Move logic for Modifier to there from printOperand
  - Use printModifiedOperand in 3 call sites that actually create
    Modifiers.
- Remove now unused Modifier parameter from printOperand
- Remove default parameter from printLeaMemReference as it only has 1
  call site that explicitly passes a parameter.
- Remove default parameter from printMemReference, make call lone call
  site explicitly pass nullptr.
- Drop Modifier parameter from printIntelMemReference, as Intel style
  memory references don't support the Modifiers in question.

This will allow future changes to printOperand() to make it a pure virtual
method on the base AsmPrinter class, allowing for more generic handling
of some architecture generic constraints. X86AsmPrinter was the only
derived class of AsmPrinter to have additional parameters on its
printOperand function.

Reviewers: craig.topper, echristo

Reviewed By: echristo

Subscribers: hiraditya, llvm-commits, srhines

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D60526

Modified:
    llvm/trunk/lib/Target/X86/X86AsmPrinter.cpp

Modified: llvm/trunk/lib/Target/X86/X86AsmPrinter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86AsmPrinter.cpp?rev=358122&r1=358121&r2=358122&view=diff
==============================================================================
--- llvm/trunk/lib/Target/X86/X86AsmPrinter.cpp (original)
+++ llvm/trunk/lib/Target/X86/X86AsmPrinter.cpp Wed Apr 10 12:01:44 2019
@@ -200,32 +200,7 @@ static void printSymbolOperand(X86AsmPri
 }
 
 static void printOperand(X86AsmPrinter &P, const MachineInstr *MI,
-                         unsigned OpNo, raw_ostream &O,
-                         const char *Modifier = nullptr);
-
-/// printPCRelImm - This is used to print an immediate value that ends up
-/// being encoded as a pc-relative value.  These print slightly differently, for
-/// example, a $ is not emitted.
-static void printPCRelImm(X86AsmPrinter &P, const MachineInstr *MI,
-                          unsigned OpNo, raw_ostream &O) {
-  const MachineOperand &MO = MI->getOperand(OpNo);
-  switch (MO.getType()) {
-  default: llvm_unreachable("Unknown pcrel immediate operand");
-  case MachineOperand::MO_Register:
-    // pc-relativeness was handled when computing the value in the reg.
-    printOperand(P, MI, OpNo, O);
-    return;
-  case MachineOperand::MO_Immediate:
-    O << MO.getImm();
-    return;
-  case MachineOperand::MO_GlobalAddress:
-    printSymbolOperand(P, MO, O);
-    return;
-  }
-}
-
-static void printOperand(X86AsmPrinter &P, const MachineInstr *MI,
-                         unsigned OpNo, raw_ostream &O, const char *Modifier) {
+                         unsigned OpNo, raw_ostream &O) {
   const MachineOperand &MO = MI->getOperand(OpNo);
   const bool IsATT = MI->getInlineAsmDialect() == InlineAsm::AD_ATT;
   switch (MO.getType()) {
@@ -233,14 +208,7 @@ static void printOperand(X86AsmPrinter &
   case MachineOperand::MO_Register: {
     if (IsATT)
       O << '%';
-    unsigned Reg = MO.getReg();
-    if (Modifier && strncmp(Modifier, "subreg", strlen("subreg")) == 0) {
-      unsigned Size = (strcmp(Modifier+6,"64") == 0) ? 64 :
-                      (strcmp(Modifier+6,"32") == 0) ? 32 :
-                      (strcmp(Modifier+6,"16") == 0) ? 16 : 8;
-      Reg = getX86SubSuperRegister(Reg, Size);
-    }
-    O << X86ATTInstPrinter::getRegisterName(Reg);
+    O << X86ATTInstPrinter::getRegisterName(MO.getReg());
     return;
   }
 
@@ -264,9 +232,52 @@ static void printOperand(X86AsmPrinter &
   }
 }
 
+
+/// printModifiedOperand - Print subregisters based on supplied modifier,
+/// deferring to printOperand() if no modifier was supplied or if operand is not
+/// a register.
+static void printModifiedOperand(X86AsmPrinter &P, const MachineInstr *MI,
+                                 unsigned OpNo, raw_ostream &O,
+                                 const char *Modifier) {
+  const MachineOperand &MO = MI->getOperand(OpNo);
+  if (!Modifier || MO.getType() != MachineOperand::MO_Register)
+    return printOperand(P, MI, OpNo, O);
+  if (MI->getInlineAsmDialect() == InlineAsm::AD_ATT)
+    O << '%';
+  unsigned Reg = MO.getReg();
+  if (strncmp(Modifier, "subreg", strlen("subreg")) == 0) {
+    unsigned Size = (strcmp(Modifier+6,"64") == 0) ? 64 :
+        (strcmp(Modifier+6,"32") == 0) ? 32 :
+        (strcmp(Modifier+6,"16") == 0) ? 16 : 8;
+    Reg = getX86SubSuperRegister(Reg, Size);
+  }
+  O << X86ATTInstPrinter::getRegisterName(Reg);
+}
+
+/// printPCRelImm - This is used to print an immediate value that ends up
+/// being encoded as a pc-relative value.  These print slightly differently, for
+/// example, a $ is not emitted.
+static void printPCRelImm(X86AsmPrinter &P, const MachineInstr *MI,
+                          unsigned OpNo, raw_ostream &O) {
+  const MachineOperand &MO = MI->getOperand(OpNo);
+  switch (MO.getType()) {
+  default: llvm_unreachable("Unknown pcrel immediate operand");
+  case MachineOperand::MO_Register:
+    // pc-relativeness was handled when computing the value in the reg.
+    printOperand(P, MI, OpNo, O);
+    return;
+  case MachineOperand::MO_Immediate:
+    O << MO.getImm();
+    return;
+  case MachineOperand::MO_GlobalAddress:
+    printSymbolOperand(P, MO, O);
+    return;
+  }
+}
+
 static void printLeaMemReference(X86AsmPrinter &P, const MachineInstr *MI,
                                  unsigned Op, raw_ostream &O,
-                                 const char *Modifier = nullptr) {
+                                 const char *Modifier) {
   const MachineOperand &BaseReg  = MI->getOperand(Op+X86::AddrBaseReg);
   const MachineOperand &IndexReg = MI->getOperand(Op+X86::AddrIndexReg);
   const MachineOperand &DispSpec = MI->getOperand(Op+X86::AddrDisp);
@@ -303,11 +314,11 @@ static void printLeaMemReference(X86AsmP
 
     O << '(';
     if (HasBaseReg)
-      printOperand(P, MI, Op+X86::AddrBaseReg, O, Modifier);
+      printModifiedOperand(P, MI, Op+X86::AddrBaseReg, O, Modifier);
 
     if (IndexReg.getReg()) {
       O << ',';
-      printOperand(P, MI, Op+X86::AddrIndexReg, O, Modifier);
+      printModifiedOperand(P, MI, Op+X86::AddrIndexReg, O, Modifier);
       unsigned ScaleVal = MI->getOperand(Op+X86::AddrScaleAmt).getImm();
       if (ScaleVal != 1)
         O << ',' << ScaleVal;
@@ -318,19 +329,18 @@ static void printLeaMemReference(X86AsmP
 
 static void printMemReference(X86AsmPrinter &P, const MachineInstr *MI,
                               unsigned Op, raw_ostream &O,
-                              const char *Modifier = nullptr) {
+                              const char *Modifier) {
   assert(isMem(*MI, Op) && "Invalid memory reference!");
   const MachineOperand &Segment = MI->getOperand(Op+X86::AddrSegmentReg);
   if (Segment.getReg()) {
-    printOperand(P, MI, Op+X86::AddrSegmentReg, O, Modifier);
+    printModifiedOperand(P, MI, Op+X86::AddrSegmentReg, O, Modifier);
     O << ':';
   }
   printLeaMemReference(P, MI, Op, O, Modifier);
 }
 
 static void printIntelMemReference(X86AsmPrinter &P, const MachineInstr *MI,
-                                   unsigned Op, raw_ostream &O,
-                                   const char *Modifier = nullptr) {
+                                   unsigned Op, raw_ostream &O) {
   const MachineOperand &BaseReg  = MI->getOperand(Op+X86::AddrBaseReg);
   unsigned ScaleVal = MI->getOperand(Op+X86::AddrScaleAmt).getImm();
   const MachineOperand &IndexReg = MI->getOperand(Op+X86::AddrIndexReg);
@@ -339,7 +349,7 @@ static void printIntelMemReference(X86As
 
   // If this has a segment register, print it.
   if (SegReg.getReg()) {
-    printOperand(P, MI, Op + X86::AddrSegmentReg, O, Modifier);
+    printOperand(P, MI, Op + X86::AddrSegmentReg, O);
     O << ':';
   }
 
@@ -347,7 +357,7 @@ static void printIntelMemReference(X86As
 
   bool NeedPlus = false;
   if (BaseReg.getReg()) {
-    printOperand(P, MI, Op + X86::AddrBaseReg, O, Modifier);
+    printOperand(P, MI, Op + X86::AddrBaseReg, O);
     NeedPlus = true;
   }
 
@@ -355,13 +365,13 @@ static void printIntelMemReference(X86As
     if (NeedPlus) O << " + ";
     if (ScaleVal != 1)
       O << ScaleVal << '*';
-    printOperand(P, MI, Op + X86::AddrIndexReg, O, Modifier);
+    printOperand(P, MI, Op + X86::AddrIndexReg, O);
     NeedPlus = true;
   }
 
   if (!DispSpec.isImm()) {
     if (NeedPlus) O << " + ";
-    printOperand(P, MI, Op + X86::AddrDisp, O, Modifier);
+    printOperand(P, MI, Op + X86::AddrDisp, O);
   } else {
     int64_t DispVal = DispSpec.getImm();
     if (DispVal || (!IndexReg.getReg() && !BaseReg.getReg())) {
@@ -510,7 +520,7 @@ bool X86AsmPrinter::PrintAsmOperand(cons
     }
   }
 
-  printOperand(*this, MI, OpNo, O, /*Modifier*/ nullptr);
+  printOperand(*this, MI, OpNo, O);
   return false;
 }
 
@@ -542,7 +552,7 @@ bool X86AsmPrinter::PrintAsmMemoryOperan
       return false;
     }
   }
-  printMemReference(*this, MI, OpNo, O);
+  printMemReference(*this, MI, OpNo, O, nullptr);
   return false;
 }
 




More information about the llvm-commits mailing list