[PATCH] D24318: AArch64: Use TTI branch functions in branch relaxation

Diana Picus via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 14 04:44:16 PDT 2016


rovka added a subscriber: rovka.
rovka accepted this revision.
rovka added a reviewer: rovka.
rovka added a comment.
This revision is now accepted and ready to land.

LGTM with minor nits.


================
Comment at: include/llvm/Target/TargetInstrInfo.h:530
@@ +529,3 @@
+  /// removed instructions.
+  virtual unsigned RemoveBranch(MachineBasicBlock &MBB,
+                                int *BytesRemoved = nullptr) const {
----------------
Since you're touching all of these, it might be a good time to fix them to follow the naming convention (removeBranch etc).

================
Comment at: include/llvm/Target/TargetInstrInfo.h:558
@@ +557,3 @@
+                                     MachineBasicBlock *DestBB,
+                                     const DebugLoc &DL) const {
+    return InsertBranch(MBB, DestBB, nullptr, ArrayRef<MachineOperand>(), DL);
----------------
I think for the sake of consistency it would be nice for this to have an optional BytesAdded operand.

================
Comment at: lib/Target/AArch64/AArch64BranchRelaxation.cpp:300
@@ +299,3 @@
+    // b   L1
+    DEBUG(dbgs() << "  Invert condition and swap its destination with ");
+
----------------
This sentence doesn't end right anymore.


https://reviews.llvm.org/D24318





More information about the llvm-commits mailing list