[PATCH] ARM64 - Conditional branch peephole

Tim Northover t.p.northover at gmail.com
Thu Oct 9 11:19:25 PDT 2014


Hi Gerolf,

This looks reasonable to me, with a couple of tweaks. I agree with most of Reanto's points too.

Cheers.

Tim.

================
Comment at: lib/Target/AArch64/AArch64InstrInfo.cpp:776
@@ +775,3 @@
+                                  const TargetRegisterInfo *TRI) {
+  // We iterate backward, starting from the instruction before CmpInstr and
+  // stop when reaching the definition of the source register or done with the
----------------
Transplanted comment is out of date.

================
Comment at: lib/Target/AArch64/AArch64InstrInfo.cpp:2942-2943
@@ +2941,4 @@
+  // instructions in between.
+  if (!modifiesConditionCode(DefMI, MI, CheckOnlyCCWrites,
+                             &getRegisterInfo())) {
+    MachineBasicBlock &MBB = *MI->getParent();
----------------
We usually prefer to bail early in failure cases:

    if (modifiesConditionCode(...))
      return false;
    [...]

================
Comment at: lib/Target/AArch64/AArch64InstrInfo.cpp:2950
@@ +2949,3 @@
+    BuildMI(MBB, MI, DL, get(AArch64::Bcc)).addImm(CC).addMBB(TBB);
+    DefMI->eraseFromParent();
+    MI->eraseFromParent();
----------------
rengolin wrote:
> Did DefMI get inserted anywhere? I may be wrong, but I don't think this is necessary
It's already going to be in the basic block, isn't it? It's not one we created.

http://reviews.llvm.org/D5637






More information about the llvm-commits mailing list