[llvm] r358236 - [X86AsmPrinter] refactor static functions into private methods. NFC

Nick Desaulniers via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 11 15:47:13 PDT 2019


Author: nickdesaulniers
Date: Thu Apr 11 15:47:13 2019
New Revision: 358236

URL: http://llvm.org/viewvc/llvm-project?rev=358236&view=rev
Log:
[X86AsmPrinter] refactor static functions into private methods. NFC

Summary:
A lot of the code for printing special cases of operands in this
translation unit are static functions. While I too have suffered many
years of abuse at the hands of C, we should prefer private methods,
particularly when you start passing around *this as your first argument,
which is a code smell.

This will help make generic vs arch specific asm printing easier, as it
brings X86AsmPrinter more in line with other arch's derived AsmPrinters.
We will then be able to more easily move architecture generic code to
the base class, and architecture specific code to the derived classes.

Some other small refactorings while we're here:
- the parameter Op is now consistently OpNo
- add spaces around binary expressions. I know we're not millionaires
  but c'mon.

Reviewers: echristo

Reviewed By: echristo

Subscribers: smeenai, hiraditya, llvm-commits, srhines, craig.topper

Tags: #llvm

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

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

Modified: llvm/trunk/lib/Target/X86/X86AsmPrinter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86AsmPrinter.cpp?rev=358236&r1=358235&r2=358236&view=diff
==============================================================================
--- llvm/trunk/lib/Target/X86/X86AsmPrinter.cpp (original)
+++ llvm/trunk/lib/Target/X86/X86AsmPrinter.cpp Thu Apr 11 15:47:13 2019
@@ -104,16 +104,16 @@ void X86AsmPrinter::EmitFunctionBodyEnd(
   }
 }
 
-/// printSymbolOperand - Print a raw symbol reference operand.  This handles
+/// PrintSymbolOperand - Print a raw symbol reference operand.  This handles
 /// jump tables, constant pools, global address and external symbols, all of
 /// which print to a label with various suffixes for relocation types etc.
-static void printSymbolOperand(X86AsmPrinter &P, const MachineOperand &MO,
-                               raw_ostream &O) {
+void X86AsmPrinter::PrintSymbolOperand(const MachineOperand &MO,
+                                       raw_ostream &O) {
   switch (MO.getType()) {
   default: llvm_unreachable("unknown symbol type!");
   case MachineOperand::MO_ConstantPoolIndex:
-    P.GetCPISymbol(MO.getIndex())->print(O, P.MAI);
-    P.printOffset(MO.getOffset(), O);
+    GetCPISymbol(MO.getIndex())->print(O, MAI);
+    printOffset(MO.getOffset(), O);
     break;
   case MachineOperand::MO_GlobalAddress: {
     const GlobalValue *GV = MO.getGlobal();
@@ -121,38 +121,37 @@ static void printSymbolOperand(X86AsmPri
     MCSymbol *GVSym;
     if (MO.getTargetFlags() == X86II::MO_DARWIN_NONLAZY ||
         MO.getTargetFlags() == X86II::MO_DARWIN_NONLAZY_PIC_BASE)
-      GVSym = P.getSymbolWithGlobalValueBase(GV, "$non_lazy_ptr");
+      GVSym = getSymbolWithGlobalValueBase(GV, "$non_lazy_ptr");
     else
-      GVSym = P.getSymbol(GV);
+      GVSym = getSymbol(GV);
 
     // Handle dllimport linkage.
     if (MO.getTargetFlags() == X86II::MO_DLLIMPORT)
-      GVSym =
-          P.OutContext.getOrCreateSymbol(Twine("__imp_") + GVSym->getName());
+      GVSym = OutContext.getOrCreateSymbol(Twine("__imp_") + GVSym->getName());
     else if (MO.getTargetFlags() == X86II::MO_COFFSTUB)
       GVSym =
-          P.OutContext.getOrCreateSymbol(Twine(".refptr.") + GVSym->getName());
+          OutContext.getOrCreateSymbol(Twine(".refptr.") + GVSym->getName());
 
     if (MO.getTargetFlags() == X86II::MO_DARWIN_NONLAZY ||
         MO.getTargetFlags() == X86II::MO_DARWIN_NONLAZY_PIC_BASE) {
-      MCSymbol *Sym = P.getSymbolWithGlobalValueBase(GV, "$non_lazy_ptr");
+      MCSymbol *Sym = getSymbolWithGlobalValueBase(GV, "$non_lazy_ptr");
       MachineModuleInfoImpl::StubValueTy &StubSym =
-          P.MMI->getObjFileInfo<MachineModuleInfoMachO>().getGVStubEntry(Sym);
+          MMI->getObjFileInfo<MachineModuleInfoMachO>().getGVStubEntry(Sym);
       if (!StubSym.getPointer())
-        StubSym = MachineModuleInfoImpl::
-          StubValueTy(P.getSymbol(GV), !GV->hasInternalLinkage());
+        StubSym = MachineModuleInfoImpl::StubValueTy(getSymbol(GV),
+                                                     !GV->hasInternalLinkage());
     }
 
     // If the name begins with a dollar-sign, enclose it in parens.  We do this
     // to avoid having it look like an integer immediate to the assembler.
     if (GVSym->getName()[0] != '$')
-      GVSym->print(O, P.MAI);
+      GVSym->print(O, MAI);
     else {
       O << '(';
-      GVSym->print(O, P.MAI);
+      GVSym->print(O, MAI);
       O << ')';
     }
-    P.printOffset(MO.getOffset(), O);
+    printOffset(MO.getOffset(), O);
     break;
   }
   }
@@ -169,13 +168,13 @@ static void printSymbolOperand(X86AsmPri
     break;
   case X86II::MO_GOT_ABSOLUTE_ADDRESS:
     O << " + [.-";
-    P.MF->getPICBaseSymbol()->print(O, P.MAI);
+    MF->getPICBaseSymbol()->print(O, MAI);
     O << ']';
     break;
   case X86II::MO_PIC_BASE_OFFSET:
   case X86II::MO_DARWIN_NONLAZY_PIC_BASE:
     O << '-';
-    P.MF->getPICBaseSymbol()->print(O, P.MAI);
+    MF->getPICBaseSymbol()->print(O, MAI);
     break;
   case X86II::MO_TLSGD:     O << "@TLSGD";     break;
   case X86II::MO_TLSLD:     O << "@TLSLD";     break;
@@ -193,14 +192,14 @@ static void printSymbolOperand(X86AsmPri
   case X86II::MO_TLVP:      O << "@TLVP";      break;
   case X86II::MO_TLVP_PIC_BASE:
     O << "@TLVP" << '-';
-    P.MF->getPICBaseSymbol()->print(O, P.MAI);
+    MF->getPICBaseSymbol()->print(O, MAI);
     break;
   case X86II::MO_SECREL:    O << "@SECREL32";  break;
   }
 }
 
-static void printOperand(X86AsmPrinter &P, const MachineInstr *MI,
-                         unsigned OpNo, raw_ostream &O) {
+void X86AsmPrinter::PrintOperand(const MachineInstr *MI, unsigned OpNo,
+                                 raw_ostream &O) {
   const MachineOperand &MO = MI->getOperand(OpNo);
   const bool IsATT = MI->getInlineAsmDialect() == InlineAsm::AD_ATT;
   switch (MO.getType()) {
@@ -221,27 +220,25 @@ static void printOperand(X86AsmPrinter &
   case MachineOperand::MO_GlobalAddress: {
     if (IsATT)
       O << '$';
-    printSymbolOperand(P, MO, O);
+    PrintSymbolOperand(MO, O);
     break;
   }
   case MachineOperand::MO_BlockAddress: {
-    MCSymbol *Sym = P.GetBlockAddressSymbol(MO.getBlockAddress());
-    Sym->print(O, P.MAI);
+    MCSymbol *Sym = GetBlockAddressSymbol(MO.getBlockAddress());
+    Sym->print(O, MAI);
     break;
   }
   }
 }
 
-
-/// printModifiedOperand - Print subregisters based on supplied modifier,
-/// deferring to printOperand() if no modifier was supplied or if operand is not
+/// 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) {
+void X86AsmPrinter::PrintModifiedOperand(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);
+    return PrintOperand(MI, OpNo, O);
   if (MI->getInlineAsmDialect() == InlineAsm::AD_ATT)
     O << '%';
   unsigned Reg = MO.getReg();
@@ -254,33 +251,32 @@ static void printModifiedOperand(X86AsmP
   O << X86ATTInstPrinter::getRegisterName(Reg);
 }
 
-/// printPCRelImm - This is used to print an immediate value that ends up
+/// 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) {
+void X86AsmPrinter::PrintPCRelImm(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);
+    PrintOperand(MI, OpNo, O);
     return;
   case MachineOperand::MO_Immediate:
     O << MO.getImm();
     return;
   case MachineOperand::MO_GlobalAddress:
-    printSymbolOperand(P, MO, O);
+    PrintSymbolOperand(MO, O);
     return;
   }
 }
 
-static void printLeaMemReference(X86AsmPrinter &P, const MachineInstr *MI,
-                                 unsigned Op, raw_ostream &O,
-                                 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);
+void X86AsmPrinter::PrintLeaMemReference(const MachineInstr *MI, unsigned OpNo,
+                                         raw_ostream &O, const char *Modifier) {
+  const MachineOperand &BaseReg = MI->getOperand(OpNo + X86::AddrBaseReg);
+  const MachineOperand &IndexReg = MI->getOperand(OpNo + X86::AddrIndexReg);
+  const MachineOperand &DispSpec = MI->getOperand(OpNo + X86::AddrDisp);
 
   // If we really don't want to print out (rip), don't.
   bool HasBaseReg = BaseReg.getReg() != 0;
@@ -302,7 +298,7 @@ static void printLeaMemReference(X86AsmP
   }
   case MachineOperand::MO_GlobalAddress:
   case MachineOperand::MO_ConstantPoolIndex:
-    printSymbolOperand(P, DispSpec, O);
+    PrintSymbolOperand(DispSpec, O);
   }
 
   if (Modifier && strcmp(Modifier, "H") == 0)
@@ -314,12 +310,12 @@ static void printLeaMemReference(X86AsmP
 
     O << '(';
     if (HasBaseReg)
-      printModifiedOperand(P, MI, Op+X86::AddrBaseReg, O, Modifier);
+      PrintModifiedOperand(MI, OpNo + X86::AddrBaseReg, O, Modifier);
 
     if (IndexReg.getReg()) {
       O << ',';
-      printModifiedOperand(P, MI, Op+X86::AddrIndexReg, O, Modifier);
-      unsigned ScaleVal = MI->getOperand(Op+X86::AddrScaleAmt).getImm();
+      PrintModifiedOperand(MI, OpNo + X86::AddrIndexReg, O, Modifier);
+      unsigned ScaleVal = MI->getOperand(OpNo + X86::AddrScaleAmt).getImm();
       if (ScaleVal != 1)
         O << ',' << ScaleVal;
     }
@@ -327,29 +323,28 @@ static void printLeaMemReference(X86AsmP
   }
 }
 
-static void printMemReference(X86AsmPrinter &P, const MachineInstr *MI,
-                              unsigned Op, raw_ostream &O,
-                              const char *Modifier) {
-  assert(isMem(*MI, Op) && "Invalid memory reference!");
-  const MachineOperand &Segment = MI->getOperand(Op+X86::AddrSegmentReg);
+void X86AsmPrinter::PrintMemReference(const MachineInstr *MI, unsigned OpNo,
+                                      raw_ostream &O, const char *Modifier) {
+  assert(isMem(*MI, OpNo) && "Invalid memory reference!");
+  const MachineOperand &Segment = MI->getOperand(OpNo + X86::AddrSegmentReg);
   if (Segment.getReg()) {
-    printModifiedOperand(P, MI, Op+X86::AddrSegmentReg, O, Modifier);
+    PrintModifiedOperand(MI, OpNo + X86::AddrSegmentReg, O, Modifier);
     O << ':';
   }
-  printLeaMemReference(P, MI, Op, O, Modifier);
+  PrintLeaMemReference(MI, OpNo, O, Modifier);
 }
 
-static void printIntelMemReference(X86AsmPrinter &P, const MachineInstr *MI,
-                                   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);
-  const MachineOperand &DispSpec = MI->getOperand(Op+X86::AddrDisp);
-  const MachineOperand &SegReg   = MI->getOperand(Op+X86::AddrSegmentReg);
+void X86AsmPrinter::PrintIntelMemReference(const MachineInstr *MI,
+                                           unsigned OpNo, raw_ostream &O) {
+  const MachineOperand &BaseReg = MI->getOperand(OpNo + X86::AddrBaseReg);
+  unsigned ScaleVal = MI->getOperand(OpNo + X86::AddrScaleAmt).getImm();
+  const MachineOperand &IndexReg = MI->getOperand(OpNo + X86::AddrIndexReg);
+  const MachineOperand &DispSpec = MI->getOperand(OpNo + X86::AddrDisp);
+  const MachineOperand &SegReg = MI->getOperand(OpNo + X86::AddrSegmentReg);
 
   // If this has a segment register, print it.
   if (SegReg.getReg()) {
-    printOperand(P, MI, Op + X86::AddrSegmentReg, O);
+    PrintOperand(MI, OpNo + X86::AddrSegmentReg, O);
     O << ':';
   }
 
@@ -357,7 +352,7 @@ static void printIntelMemReference(X86As
 
   bool NeedPlus = false;
   if (BaseReg.getReg()) {
-    printOperand(P, MI, Op + X86::AddrBaseReg, O);
+    PrintOperand(MI, OpNo + X86::AddrBaseReg, O);
     NeedPlus = true;
   }
 
@@ -365,13 +360,13 @@ static void printIntelMemReference(X86As
     if (NeedPlus) O << " + ";
     if (ScaleVal != 1)
       O << ScaleVal << '*';
-    printOperand(P, MI, Op + X86::AddrIndexReg, O);
+    PrintOperand(MI, OpNo + X86::AddrIndexReg, O);
     NeedPlus = true;
   }
 
   if (!DispSpec.isImm()) {
     if (NeedPlus) O << " + ";
-    printOperand(P, MI, Op + X86::AddrDisp, O);
+    PrintOperand(MI, OpNo + X86::AddrDisp, O);
   } else {
     int64_t DispVal = DispSpec.getImm();
     if (DispVal || (!IndexReg.getReg() && !BaseReg.getReg())) {
@@ -457,13 +452,13 @@ bool X86AsmPrinter::PrintAsmOperand(cons
       case MachineOperand::MO_ExternalSymbol:
         llvm_unreachable("unexpected operand type!");
       case MachineOperand::MO_GlobalAddress:
-        printSymbolOperand(*this, MO, O);
+        PrintSymbolOperand(MO, O);
         if (Subtarget->isPICStyleRIPRel())
           O << "(%rip)";
         return false;
       case MachineOperand::MO_Register:
         O << '(';
-        printOperand(*this, MI, OpNo, O);
+        PrintOperand(MI, OpNo, O);
         O << ')';
         return false;
       }
@@ -471,7 +466,7 @@ bool X86AsmPrinter::PrintAsmOperand(cons
     case 'c': // Don't print "$" before a global var name or constant.
       switch (MO.getType()) {
       default:
-        printOperand(*this, MI, OpNo, O);
+        PrintOperand(MI, OpNo, O);
         break;
       case MachineOperand::MO_Immediate:
         O << MO.getImm();
@@ -481,7 +476,7 @@ bool X86AsmPrinter::PrintAsmOperand(cons
       case MachineOperand::MO_ExternalSymbol:
         llvm_unreachable("unexpected operand type!");
       case MachineOperand::MO_GlobalAddress:
-        printSymbolOperand(*this, MO, O);
+        PrintSymbolOperand(MO, O);
         break;
       }
       return false;
@@ -489,7 +484,7 @@ bool X86AsmPrinter::PrintAsmOperand(cons
     case 'A': // Print '*' before a register (it must be a register)
       if (MO.isReg()) {
         O << '*';
-        printOperand(*this, MI, OpNo, O);
+        PrintOperand(MI, OpNo, O);
         return false;
       }
       return true;
@@ -502,11 +497,11 @@ bool X86AsmPrinter::PrintAsmOperand(cons
     case 'V': // Print native register without '%'
       if (MO.isReg())
         return printAsmMRegister(*this, MO, ExtraCode[0], O);
-      printOperand(*this, MI, OpNo, O);
+      PrintOperand(MI, OpNo, O);
       return false;
 
     case 'P': // This is the operand of a call, treat specially.
-      printPCRelImm(*this, MI, OpNo, O);
+      PrintPCRelImm(MI, OpNo, O);
       return false;
 
     case 'n': // Negate the immediate or print a '-' before the operand.
@@ -520,7 +515,7 @@ bool X86AsmPrinter::PrintAsmOperand(cons
     }
   }
 
-  printOperand(*this, MI, OpNo, O);
+  PrintOperand(MI, OpNo, O);
   return false;
 }
 
@@ -528,7 +523,7 @@ bool X86AsmPrinter::PrintAsmMemoryOperan
                                           const char *ExtraCode,
                                           raw_ostream &O) {
   if (MI->getInlineAsmDialect() == InlineAsm::AD_Intel) {
-    printIntelMemReference(*this, MI, OpNo, O);
+    PrintIntelMemReference(MI, OpNo, O);
     return false;
   }
 
@@ -545,14 +540,14 @@ bool X86AsmPrinter::PrintAsmMemoryOperan
       // These only apply to registers, ignore on mem.
       break;
     case 'H':
-      printMemReference(*this, MI, OpNo, O, "H");
+      PrintMemReference(MI, OpNo, O, "H");
       return false;
     case 'P': // Don't print @PLT, but do print as memory.
-      printMemReference(*this, MI, OpNo, O, "no-rip");
+      PrintMemReference(MI, OpNo, O, "no-rip");
       return false;
     }
   }
-  printMemReference(*this, MI, OpNo, O, nullptr);
+  PrintMemReference(MI, OpNo, O, nullptr);
   return false;
 }
 

Modified: llvm/trunk/lib/Target/X86/X86AsmPrinter.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86AsmPrinter.h?rev=358236&r1=358235&r2=358236&view=diff
==============================================================================
--- llvm/trunk/lib/Target/X86/X86AsmPrinter.h (original)
+++ llvm/trunk/lib/Target/X86/X86AsmPrinter.h Thu Apr 11 15:47:13 2019
@@ -102,6 +102,18 @@ class LLVM_LIBRARY_VISIBILITY X86AsmPrin
   // Choose between emitting .seh_ directives and .cv_fpo_ directives.
   void EmitSEHInstruction(const MachineInstr *MI);
 
+  void PrintSymbolOperand(const MachineOperand &MO, raw_ostream &O);
+  void PrintOperand(const MachineInstr *MI, unsigned OpNo, raw_ostream &O);
+  void PrintModifiedOperand(const MachineInstr *MI, unsigned OpNo,
+                            raw_ostream &O, const char *Modifier);
+  void PrintPCRelImm(const MachineInstr *MI, unsigned OpNo, raw_ostream &O);
+  void PrintLeaMemReference(const MachineInstr *MI, unsigned OpNo,
+                            raw_ostream &O, const char *Modifier);
+  void PrintMemReference(const MachineInstr *MI, unsigned OpNo, raw_ostream &O,
+                         const char *Modifier);
+  void PrintIntelMemReference(const MachineInstr *MI, unsigned OpNo,
+                              raw_ostream &O);
+
 public:
   X86AsmPrinter(TargetMachine &TM, std::unique_ptr<MCStreamer> Streamer);
 




More information about the llvm-commits mailing list