[MSP430AsmPrinter] Refactor special cases out of printOperand. NFC

[nickdesaulniers]

Summary

Refactoring to make changing the interface later simpler (trying to
remove Modifier from printOperand).

printOperand has an ominous comment for MO_GlobalAddress, but then no
unit tests (from what I can tell). Add some that pass before and after
changes, for fearless refactoring.

There is no "mem" modifier, so isMemOp is always false.

There's only special handling in the method printSrcMemOperand, so move
special logic there, simplifying printOperand.

[echristo]

Kristina has been in here the most, adding.

[nickdesaulniers]

Superseded by: https://reviews.llvm.org/D60887

llvm/lib/Target/MSP430/MSP430AsmPrinter.cpp

namespace {
   public:
     MSP430AsmPrinter(TargetMachine &TM, std::unique_ptr<MCStreamer> Streamer)
         : AsmPrinter(TM, std::move(Streamer)) {}
 
     StringRef getPassName() const override { return "MSP430 Assembly Printer"; }
 
     bool runOnMachineFunction(MachineFunction &MF) override;
 
-    void printOperand(const MachineInstr *MI, int OpNum,
-                      raw_ostream &O, const char* Modifier = nullptr);
+    void PrintGlobalOperand(const MachineOperand &MO, raw_ostream &O);
+    void printOperand(const MachineInstr *MI, int OpNum, raw_ostream &O);
     void printSrcMemOperand(const MachineInstr *MI, int OpNum,
                             raw_ostream &O);
     bool PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
                          const char *ExtraCode, raw_ostream &O) override;
     bool PrintAsmMemoryOperand(const MachineInstr *MI, unsigned OpNo,
                                const char *ExtraCode, raw_ostream &O) override;
     void EmitInstruction(const MachineInstr *MI) override;
 
     void EmitInterruptVectorSection(MachineFunction &ISR);
   };
 } // end of anonymous namespace
 
+void MSP430AsmPrinter::PrintGlobalOperand(const MachineOperand &MO,
+                                          raw_ostream &O) {
+    assert(MO.getType() == MachineOperand::MO_GlobalAddress);
+    uint64_t Offset = MO.getOffset();
+    if (Offset)
+      O << '(' << Offset << '+';
+
+    getSymbol(MO.getGlobal())->print(O, MAI);
+
+    if (Offset)
+      O << ')';
+}
 
 void MSP430AsmPrinter::printOperand(const MachineInstr *MI, int OpNum,
-                                    raw_ostream &O, const char *Modifier) {
+                                    raw_ostream &O) {
   const MachineOperand &MO = MI->getOperand(OpNum);
   switch (MO.getType()) {
   default: llvm_unreachable("Not implemented yet!");
   case MachineOperand::MO_Register:
     O << MSP430InstPrinter::getRegisterName(MO.getReg());
     return;
   case MachineOperand::MO_Immediate:
-    if (!Modifier || strcmp(Modifier, "nohash"))
-      O << '#';
-    O << MO.getImm();
+    O << '#' << MO.getImm();
     return;
   case MachineOperand::MO_MachineBasicBlock:
     MO.getMBB()->getSymbol()->print(O, MAI);
     return;
   case MachineOperand::MO_GlobalAddress: {
-    bool isMemOp  = Modifier && !strcmp(Modifier, "mem");
-    uint64_t Offset = MO.getOffset();
-
     // If the global address expression is a part of displacement field with a
     // register base, we should not emit any prefix symbol here, e.g.
     //   mov.w &foo, r1
     // vs
     //   mov.w glb(r1), r2
     // Otherwise (!) msp430-as will silently miscompile the output :(
-    if (!Modifier || strcmp(Modifier, "nohash"))
-      O << (isMemOp ? '&' : '#');
-    if (Offset)
-      O << '(' << Offset << '+';
-
-    getSymbol(MO.getGlobal())->print(O, MAI);
-
-    if (Offset)
-      O << ')';
-
+    O << '#';
+    PrintGlobalOperand(MO, O);
     return;
   }
   }
 }
 
 void MSP430AsmPrinter::printSrcMemOperand(const MachineInstr *MI, int OpNum,
                                           raw_ostream &O) {
   const MachineOperand &Base = MI->getOperand(OpNum);
   const MachineOperand &Disp = MI->getOperand(OpNum+1);
 
   // Print displacement first
 
+  if (Disp.isImm()) {
     // Imm here is in fact global address - print extra modifier.
-  if (Disp.isImm() && Base.getReg() == MSP430::SR)
+    if (Base.getReg() == MSP430::SR)
       O << '&';
-  printOperand(MI, OpNum+1, O, "nohash");
+    O << Disp.getImm();
+  } else if (Disp.isGlobal())
+    PrintGlobalOperand(Disp, O);
+  else
+    printOperand(MI, OpNum + 1, O);
 
   // Print register base field
   if (Base.getReg() != MSP430::SR && Base.getReg() != MSP430::PC) {
     O << '(';
     printOperand(MI, OpNum, O);
     O << ')';
   }
 }

llvm/test/CodeGen/MSP430/inline-asm-absolute-addressing.ll

 ; RUN: llc < %s | FileCheck %s
 
 ; Check that absolute addressing mode is represented in a way
 ; defined in MSP430 EABI and not as indexed addressing mode form.
 ; See PR39993 for details.
 
 target datalayout = "e-p:16:8:8-i8:8:8-i16:8:8-i32:8:8-n8:16"
 target triple = "msp430-elf"
 
 define void @f() {
 entry:
 ; CHECK: mov r1, &256
   call void asm sideeffect "mov r1, $0", "*m"(i8* inttoptr (i16 256 to i8*))
   ret void
 }
+
+@baz = internal global i32 0, align 4
+
+; CHECK-LABEL: foo
+; CHECK: mov &baz, r1
+define dso_local void @foo() {
+  call void asm sideeffect "mov.w &baz, r1", ""()
+  ret void
+}
+
+; CHECK-LABEL: bar
+; CHECK: mov baz(r1), r2
+define dso_local void @bar() {
+  call void asm sideeffect "mov.w baz(r1), r2", ""()
+  ret void
+}
+
+; CHECK-LABEL: quux
+; CHECK: mov #baz, r1
+define dso_local void @quux() {
+  call void asm sideeffect "mov.w #baz, r1", ""()
+  ret void
+}
+
+; CHECK-LABEL: foobar
+; CHECK: mov #baz, r1
+define dso_local void @foobar() {
+  call void asm sideeffect "mov.w $0, r1", "i"(i32* nonnull @baz)
+  ret void
+}
+
+; CHECK-LABEL: foobarbaz
+; CHECK: call #foobar
+define dso_local void @foobarbaz() {
+  call void asm sideeffect "call #foobar", ""()
+  ret void
+}
+
+; CHECK-LABEL: foobarbazquux
+; CHECK: mov #baz, 0(r1)
+define dso_local void @foobarbazquux() {
+  call void asm sideeffect "mov.w $0, r1", "m"(i32* nonnull @baz)
+  ret void
+}