[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

Fri Aug 9 18:56:00 PDT 2019

jcai19 marked 4 inline comments as done.
jcai19 added a comment.

@efriedma I have changed my implementation to lower llvm.gnu.eabi.mcount intrinsic into pseudo instructions directly, instead of first lowering them into SelectionDAG call nodes. Thanks.

================
Comment at: llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp:1156
   MachineInstr &MI = *MBBI;
+  LLVM_DEBUG(dbgs() << "ARMExpandPseudo::ExpandMI: " << MI << "\n");
   unsigned Opcode = MI.getOpcode();
----------------
kristof.beyls wrote:
> I wonder whether this is a good debug printing line to commit?
> IIUC, this will print every MI instruction that gets looked at by ArmExpandPseudo.
> I would imagine that that could produce too much noise. It'd be more interesting if only the MIs that actually got transformed would be printed.
> But maybe best to just not add this debug printing line in this patch?
Sorry, I forgot to remove it. I was using it to debug my change locally.

================
Comment at: llvm/test/CodeGen/ARM/gnu_mcount_nc.ll:1-2
+; RUN: llc -mtriple=armv7a-linux-gnueabihf %s -o - | FileCheck %s --check-prefix=CHECK-ARM
+; RUN: llc -mtriple=thumbv7a-linux-gnueabihf %s -o - | FileCheck %s --check-prefix=CHECK-THUMB
+
----------------
kristof.beyls wrote:
> Given that the push-lr transform only gets implemented for DAGISel (IIUC), maybe it'd be useful to also have test run lines that check the correct thing happens when using fastisel and globalisel (presumably by falling back to DAGISel)?
That's a good point. Checks added for fast-isel and global-isel.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65019