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

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added inline comments.


================
Comment at: llvm/lib/Target/M68k/M68kCallingConv.h:55
+
+  // FIXME: This is probably wrong
+  auto I = CCInfo.F.arg_begin();
----------------
jrtc27 wrote:
> ... because?
Deleting the comment doesn't fix the issue, unless the comment itself was wrong.


================
Comment at: llvm/lib/Target/M68k/M68kISelLowering.cpp:217-218
+      Chain, DL, Dst, Src, SizeNode, Flags.getNonZeroByValAlign(),
+      /*isVolatile*/false, /*AlwaysInline=*/true,
+      /*isTailCall*/false, MachinePointerInfo(), MachinePointerInfo());
+}
----------------
Still missing = signs for 2/3


================
Comment at: llvm/lib/Target/M68k/M68kTargetMachine.cpp:36
+
+// FIXME #28 this layout is true for M68000 original cpu, other variants will
+// affect DL computation
----------------
myhsu wrote:
> rengolin wrote:
> > Are these github issues? If so, it would be nice to replace them with a proper explanation. When moving to the llvm repo, the connection will disappear.
> Yes...these numbers were coming from the legacy repo. Will bring some useful context from those issues if there is any
DataLayout is ABI, you can't change it based on the CPU if you want ABI compatibility (with the exception of optionally adding support for additional types).


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

https://reviews.llvm.org/D88391