[PATCH] D40786: [mips] Add partial support for R6 in the long branch pass

Simon Atanasyan via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 6 02:47:59 PST 2017


atanasyan accepted this revision.
atanasyan added a comment.
This revision is now accepted and ready to land.

LGTM with a few nits



================
Comment at: lib/Target/Mips/MipsLongBranch.cpp:353
+              .addMBB(BalTgtMBB);
+      if (!Subtarget.hasMips32r6()) {
+        LongBrMBB->insert(Pos, BalInstr);
----------------
I would revert this condition:
```
if (Subtarget.hasMips32r6()) {
  LongBrMBB->insert(Pos, ADDiuInstr);
  LongBrMBB->insert(Pos, BalInstr);
} else {
  LongBrMBB->insert(Pos, BalInstr);
  LongBrMBB->insert(Pos, ADDiuInstr);
  LongBrMBB->rbegin()->bundleWithPred();
}
```


================
Comment at: lib/Target/Mips/MipsLongBranch.cpp:462
+              .addMBB(BalTgtMBB);
+      if (!Subtarget.hasMips32r6()) {
+        LongBrMBB->insert(Pos, BalInstr);
----------------
And here.


================
Comment at: lib/Target/Mips/MipsLongBranch.cpp:504
     LongBrMBB->addSuccessor(TgtMBB);
-    MIBundleBuilder(*LongBrMBB, Pos)
-      .append(BuildMI(*MF, DL, TII->get(Mips::J)).addMBB(TgtMBB))
-      .append(BuildMI(*MF, DL, TII->get(Mips::NOP)));
+    if (!Subtarget.hasMips32r6())
+      MIBundleBuilder(*LongBrMBB, Pos)
----------------
And here.


================
Comment at: lib/Target/Mips/MipsLongBranch.cpp:547
 
-  LongBranchSeqSize =
-      !IsPIC ? 2 : (ABI.IsN64() ? 10 : (!STI.isTargetNaCl() ? 9 : 10));
+  LongBranchSeqSize = !IsPIC
+                          ? (STI.hasMips32r6() ? 1 : 2)
----------------
I would reduce number of ternary operators a bit:
```
LongBranchSeqSize = IsPIC
    ? ((ABI.IsN64() || STI.isTargetNaCl()) ? 10 : 9)
    : (STI.hasMips32r6() ? 1 : 2);
```


Repository:
  rL LLVM

https://reviews.llvm.org/D40786





More information about the llvm-commits mailing list