[PATCH] D59635: [AArch64] NFC: Cleanup isAArch64FrameOffsetLegal

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 21 12:42:34 PDT 2019


efriedma added inline comments.


================
Comment at: lib/Target/AArch64/AArch64InstrInfo.cpp:2129
+  case AArch64::LDRSHWui:
+  case AArch64::LDRSHXui:
   case AArch64::STRHui:
----------------
It seems unlikely to me that this actually has no effect on the generated code; getMemOpInfo has other callers.


================
Comment at: lib/Target/AArch64/AArch64InstrInfo.cpp:3278
+
+  Offset += Remainder;
   if (OutUseUnscaledOp)
----------------
The math here is weird... could you check whether the offset is valid in some more obvious way?


================
Comment at: lib/Target/AArch64/AArch64InstrInfo.h:91
+  /// Returns the index for the immediate for a given instruction.
+  static unsigned getImmIdx(unsigned Opc);
+
----------------
This could use a better name, to reflect how you expect it to be used, e.g. getLoadStoreImmIdx?  (It probably isn't right for all instructions...)


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

https://reviews.llvm.org/D59635





More information about the llvm-commits mailing list