[PATCH] D62667: [ARM] Add the non-MVE instructions in Arm v8.1-M.

[Sam Parker via Phabricator via llvm-commits]

samparker added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMScheduleA57.td:98
+
+  let UnsupportedFeatures = [HasV8_1MMainline, HasMVEInt, HasMVEFloat,
+                             HasFPRegsV8_1M];
----------------
Why is this needed? And why just the A57?


================
Comment at: llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp:5754
+
+static DecodeStatus DecodeVSTRVLDR_SYSREG_post(MCInst &Inst, unsigned Val,
+                                               uint64_t Address,
----------------
Looks like these two functions could be refactored into one.


================
Comment at: llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp:861
+  }
+  case ARM::fixup_bfc_target: {
+    const char *FixupDiagnostic = reasonForFixupRelaxation(Fixup, Value);
----------------
This is a lot of copy-paste, these three cases can be folded into one.


================
Comment at: llvm/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp:955
+                   const MCSubtargetInfo &STI) const {
+  // FIXME: The immediate operand should have already been encoded like this
+  // before ever getting here. The encoder method should just need to combine
----------------
So does this mean we just haven't created, and selected, an operand properly? Why?


================
Comment at: llvm/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp:1034
+
+  // FIXME: The immediate operand should have already been encoded like this
+  // before ever getting here. The encoder method should just need to combine
----------------
Same question here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62667