[PATCH] D154899: [SystemZ] Allow symbols in immediate asm operands

[Ulrich Weigand via Phabricator via llvm-commits]

uweigand added a comment.

Thanks for working on that, Ilya!   If we do allow symbols in immediates (and I agree we should), we should then also handle the case where they don't completely resolve, but result in relocations.  This should be straightforward to add once we already have fixups.   In addition to `R_S390_12` (which is already emitted, and should be used for the `U12Imm` cases as well), we do support 1-, 2-, 4-, and  8-byte relocations.   The sub-byte (U1Imm, U2Imm, U3Imm, U4Imm) and .insn opcode (U48Imm) cases don't have corresponding relocations, but I believe for those we should not allow any symbolic expressions anyway.

Also, it would be good to have tests for the PC-relative cases (`symbol-.`), both where those resolve at assemble time (symbol in the current section), and where a PC-relative reloc is required (symbol in another section).

Finally, for the tablegen changes I'd prefer to have someone more familiar with that code base to review it.   Maybe we can split this off into a separate diff (possibly combined with a `getDispOpValue` change to make use of it for displacements)?



================
Comment at: llvm/lib/Target/SystemZ/MCTargetDesc/SystemZInstPrinter.cpp:88
+  if (MO.isExpr()) {
+    O << markup("<imm:") << *MO.getExpr() << markup(">");
+    return;
----------------
I don't think we should have the "imm" markup for expressions.


================
Comment at: llvm/lib/Target/SystemZ/MCTargetDesc/SystemZMCCodeEmitter.cpp:207
+    return 0;
+  }
   llvm_unreachable("Unexpected operand type!");
----------------
I think this should rather go into `getDispOpValue` ... a displacement is in fact simply an immediate.

The `getOperandBitOffset` logic should allow us to replace the `MemOpsEmitted` hack we currently have there, and then generalize it to all immediates.

The `getMachineOpValue` will then be only used for registers.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154899/new/

https://reviews.llvm.org/D154899