[PATCH] D88391: [M68k] (Patch 5/8) Target lowering

[Min-Yih Hsu via Phabricator via llvm-commits]

myhsu added inline comments.


================
Comment at: llvm/lib/Target/M68k/M68kISelLowering.h:35
+
+  CALL,
+  RET,
----------------
jrtc27 wrote:
> ` = FIRST_NUMBER`?
Looking at other Targets' implementations, I think `FIRST_NUMBER` should always be `ISD::BUILTIN_OP_END` instead of the first target-specific ISD type.


================
Comment at: llvm/lib/Target/M68k/M68kISelLowering.h:82
+
+  GlobalBaseReg,
+
----------------
jrtc27 wrote:
> Change of naming style
Well...since X86 has the same naming style for `GlobalBaseReg`, I'm incline not to change it to be in consistent with X86


================
Comment at: llvm/lib/Target/M68k/M68kISelLowering.h:95
+  // falls back to heap allocation if not.
+  SEG_ALLOCA,
+};
----------------
jrtc27 wrote:
> Corresponding `LAST_NUMBER`?
Taking other Targets as references, I don't think we need to assign `LAST_NUMBER` here


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

https://reviews.llvm.org/D88391