[PATCH] D64836: [Xtensa 10/10] Add relaxations and fixups. Add rest part of Xtensa Core Instructions.

[Sergei Barannikov via Phabricator via llvm-commits]

barannikov88 added a comment.

The patch is too big, I wasn't able to review it thoroughly.



================
Comment at: llvm/lib/Target/Xtensa/Disassembler/XtensaDisassembler.cpp:107
+static DecodeStatus decodeCallOperand(MCInst &Inst, uint64_t Imm,
+                                      int64_t Address, const void *Decoder) {
+  assert(isUInt<18>(Imm) && "Invalid immediate");
----------------
(minor) I believe Address should be uint64_t (here and in other methods below).


================
Comment at: llvm/lib/Target/Xtensa/Disassembler/XtensaDisassembler.cpp:121
+static DecodeStatus decodeBranchOperand(MCInst &Inst, uint64_t Imm,
+                                        int64_t Address, const void *Decoder) {
+  switch (Inst.getOpcode()) {
----------------
(minor) All these method can accept `const MCDisassebler *Decoder` instead of `const void *`.



================
Comment at: llvm/lib/Target/Xtensa/MCTargetDesc/XtensaELFObjectWriter.cpp:50
+
+  switch ((unsigned)Fixup.getKind()) {
+  case FK_Data_4:
----------------
(note for the future?) This should take into account IsPCRel flag.


================
Comment at: llvm/lib/Target/Xtensa/MCTargetDesc/XtensaInstPrinter.cpp:101-104
+  } else if (MC.isExpr())
+    MC.getExpr()->print(OS, &MAI, true);
+  else
+    llvm_unreachable("Invalid operand");
----------------
Either all of 'if's should have braces or none.


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

https://reviews.llvm.org/D64836