[AVR][NFC] Refactor 'AVRAsmPrinter::PrintAsmOperand'

[benshi001]

llvm/lib/Target/AVR/AVRAsmPrinter.cpp

113

We should not use assert here, since the case we checked is user illegal input, it would be better to return true and let the llvm framework to give the error message.

And I combine the check with the above if check.

[benshi001]

Though the title is refactor, actually the part of handling extra code is left unchanged. I only make the high run flow more clear.

[benshi001]

llvm/lib/Target/AVR/AVRAsmPrinter.cpp

112

These conditions are checked in https://github.com/llvm/llvm-project/blob/main/llvm/test/CodeGen/AVR/inline-asm/inline-asm-invalid.ll

But I can not compose tests for the following two cases:

1. ExtraCode[1] != 0, which will trigger to a fatal error in llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp

2. The operand is not a register but ExtraCode[0] is not empty. Maybe we treat it as an input error (return true) is better than an assert.

[Chenbing.Zheng]

llvm/lib/Target/AVR/AVRAsmPrinter.cpp

111

It seems that ExtraCode[0] < 'A' || ExtraCode[0] > 'Z' will return false according to the original logic ?

[benshi001]

llvm/lib/Target/AVR/AVRAsmPrinter.cpp

111

The orginal logic is wrong. Returning false means success, while true means failure. This is the logic of https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/CodeGen/AsmPrinter.h#L787

[Chenbing.Zheng]

LGTM

llvm/lib/Target/AVR/AVRAsmPrinter.cpp

default:
     llvm_unreachable("Not implemented yet!");
   }
 }
 
 bool AVRAsmPrinter::PrintAsmOperand(const MachineInstr *MI, unsigned OpNum,
                                     const char *ExtraCode, raw_ostream &O) {
   // Default asm printer can only deal with some extra codes,
   // so try it first.
-  bool Error = AsmPrinter::PrintAsmOperand(MI, OpNum, ExtraCode, O);
+  if (!AsmPrinter::PrintAsmOperand(MI, OpNum, ExtraCode, O))
+    return false;
 
-  if (Error && ExtraCode && ExtraCode[0]) {
-    if (ExtraCode[1] != 0)
-      return true; // Unknown modifier.
-
-    if (ExtraCode[0] >= 'A' && ExtraCode[0] <= 'Z') {
-      const MachineOperand &RegOp = MI->getOperand(OpNum);
-
-      assert(RegOp.isReg() && "Operand must be a register when you're"
-                              "using 'A'..'Z' operand extracodes.");
-      Register Reg = RegOp.getReg();
+  const MachineOperand &MO = MI->getOperand(OpNum);
 
-      unsigned ByteNumber = ExtraCode[0] - 'A';
+  if (ExtraCode && ExtraCode[0]) {
+    // Unknown extra code.
+    if (ExtraCode[1] != 0 || ExtraCode[0] < 'A' || ExtraCode[0] > 'Z')
+      return true;
+
+    // Operand must be a register when using 'A' ~ 'Z' extra code.
+    if (!MO.isReg())
+      return true;
 
-      unsigned OpFlags = MI->getOperand(OpNum - 1).getImm();
-      unsigned NumOpRegs = InlineAsm::getNumOperandRegisters(OpFlags);
-      (void)NumOpRegs;
+    Register Reg = MO.getReg();
+
+    unsigned ByteNumber = ExtraCode[0] - 'A';
 
-      const AVRSubtarget &STI = MF->getSubtarget<AVRSubtarget>();
-      const TargetRegisterInfo &TRI = *STI.getRegisterInfo();
 
-      const TargetRegisterClass *RC = TRI.getMinimalPhysRegClass(Reg);
-      unsigned BytesPerReg = TRI.getRegSizeInBits(*RC) / 8;
-      assert(BytesPerReg <= 2 && "Only 8 and 16 bit regs are supported.");
 
-      unsigned RegIdx = ByteNumber / BytesPerReg;
-      if (RegIdx >= NumOpRegs)
-        return true;
-      Reg = MI->getOperand(OpNum + RegIdx).getReg();
 
-      if (BytesPerReg == 2) {
-        Reg = TRI.getSubReg(Reg, ByteNumber % BytesPerReg ? AVR::sub_hi
-                                                          : AVR::sub_lo);
-      }
+      Reg = TRI.getSubReg(Reg,
+                          ByteNumber % BytesPerReg ? AVR::sub_hi : AVR::sub_lo);
 
-      O << AVRInstPrinter::getPrettyRegisterName(Reg, MRI);
-      return false;
-    }
-  }
-
-  // Print global symbols.
-  const auto &MO = MI->getOperand(OpNum);
-  if (Error && MO.getType() == MachineOperand::MO_GlobalAddress) {
-    PrintSymbolOperand(MO, O);
-    return false;
-  }
 
-  if (Error)
-    printOperand(MI, OpNum, O);
+  if (MO.getType() == MachineOperand::MO_GlobalAddress)
+    PrintSymbolOperand(MO, O); // Print global symbols.
+  else
+    printOperand(MI, OpNum, O); // Fallback to ordinary cases.
 
   return false;
 }
 
 bool AVRAsmPrinter::PrintAsmMemoryOperand(const MachineInstr *MI,
                                           unsigned OpNum, const char *ExtraCode,
                                           raw_ostream &O) {
   if (ExtraCode && ExtraCode[0])