[TableGen] Fix printing second PC-relative operand

[ikudrin]

Summary

If an instruction has several operands and a PC-relative one is not the first of them, the generator may produce the code that does not pass the Address parameter to the printout method. For example, for an Arm instruction LE LR, $imm, it reuses the same code as for other instructions where the second operand is not PC-relative:

void ARMInstPrinter::printInstruction(...) {
...
  case 11:
    // BF16VDOTI_VDOTD, BF16VDOTI_VDOTQ, BF16VDOTS_VDOTD, ...
    printOperand(MI, 1, STI, O);
    O << ", ";
    printOperand(MI, 2, STI, O);
    break;
...

The patch fixes that by considering PCRel when comparing AsmWriterOperand values.

[MaskRay]

I know little about llvm/utils/TableGen/AsmWriterEmitter.cpp, but after debugging it a bit, I think this is reasonable.

llvm/test/TableGen/AsmWriterPCRelOp.td

32

// CHECK-LABEL: ArchInstPrinter::printInstruction(

[MaskRay]

llvm/test/TableGen/AsmWriterPCRelOp.td

1

%t.log => | FileCheck %s

[ikudrin]

llvm/test/TableGen/AsmWriterPCRelOp.td

1

Thanks a lot! I'll fix that on commit.

32

I am afraid I am not following you. What is the difference and how CHECK-LABEL can be useful in this case?

[MaskRay]

llvm/test/TableGen/AsmWriterPCRelOp.td

32

When there are multiple functions, -LABEL patterns are matched first and they break the input into multiple regions. The FileCheck failing diagnostic can be slightly better with -LABEL. For this case I guess -LABEL may be an overkill. Do your original pattern is fine.

[ikudrin]

llvm/test/TableGen/AsmWriterPCRelOp.td

32

Thanks for explaining. I'll stick with my original check then.

llvm/test/TableGen/AsmWriterPCRelOp.td

+// RUN: llvm-tblgen -gen-asm-writer -I %p/../../include %s > %t.log
+
+include "llvm/Target/Target.td"
+
+def ArchInstrInfo : InstrInfo { }
+
+def Arch : Target {
+  let InstructionSet = ArchInstrInfo;
+}
+
+def R0 : Register<"r0">;
+def Reg : RegisterClass<"Reg", [i32], 0, (add R0)>;
+
+def IntOperand: Operand<i32>;
+
+def PCRelOperand : Operand<i32> {
+  let OperandType = "OPERAND_PCREL";
+}
+
+def foo : Instruction {
+  let OutOperandList = (outs);
+  let InOperandList = (ins Reg:$reg, IntOperand:$imm);
+  let AsmString = "foo $reg, $imm";
+}
+
+def bar : Instruction {
+  let OutOperandList = (outs);
+  let InOperandList = (ins Reg:$reg, PCRelOperand:$imm);
+  let AsmString = "bar $reg, $imm";
+}
+
+// CHECK:      ArchInstPrinter::printInstruction
+// CHECK:      // bar, foo
+// CHECK-NEXT: printOperand(MI, 0, O);
+// CHECK:      // foo
+// CHECK-NEXT: printOperand(MI, 1, O);
+// CHECK:      // bar
+// CHECK-NEXT: printOperand(MI, Address, 1, O);

llvm/utils/TableGen/AsmWriterInst.h

AsmWriterOperand(const std::string &Printer, unsigned _MIOpNo,
                      const std::string &Modifier,
                      OpType op = isMachineInstrOperand, bool PCRel = false)
         : OperandType(op), MIOpNo(_MIOpNo), Str(Printer), MiModifier(Modifier),
           PCRel(PCRel) {}
 
     bool operator!=(const AsmWriterOperand &Other) const {
       if (OperandType != Other.OperandType || Str != Other.Str) return true;
       if (OperandType == isMachineInstrOperand)
-        return MIOpNo != Other.MIOpNo || MiModifier != Other.MiModifier;
+        return MIOpNo != Other.MIOpNo || MiModifier != Other.MiModifier ||
+               PCRel != Other.PCRel;
       return false;
     }
     bool operator==(const AsmWriterOperand &Other) const {
       return !operator!=(Other);
     }
 
     /// getCode - Return the code that prints this operand.
     std::string getCode(bool PassSubtarget) const;