[PATCH] D46641: [mips] WIP: Merge MipsLongBranch and MipsHazardSchedule passes

[Simon Dardis via Phabricator via llvm-commits]

sdardis added inline comments.


================
Comment at: lib/Target/Mips/MipsBranchExpansion.cpp:141
+
+  StringRef getPassName() const override { return "Mips Long Branch and Hazard Pass"; }
+
----------------
"Mips Branch Expansion Pass"


================
Comment at: lib/Target/Mips/MipsBranchExpansion.cpp:368-371
+  const MipsSubtarget &Subtarget =
+      static_cast<const MipsSubtarget &>(MFp->getSubtarget());
+  const MipsInstrInfo *TII =
+      static_cast<const MipsInstrInfo *>(Subtarget.getInstrInfo());
----------------
These two can be lifted to be private class members, so all functions can access them.


================
Comment at: lib/Target/Mips/MipsBranchExpansion.cpp:708-711
+  if (IsPIC && static_cast<const MipsTargetMachine &>(TM).getABI().IsO32() &&
+      MF.getInfo<MipsFunctionInfo>()->globalBaseRegSet() && !isGPDispEmitted)
+    emitGPDisp(MF, TII);
+  isGPDispEmitted = true;
----------------
This hunk should be part of the runOnMachineFunction function, rather than here.


================
Comment at: lib/Target/Mips/MipsTargetMachine.cpp:298-303
+  // MipsLongBranchPass and MipsHazardSchedule passes are joined to one
+  // pass because of mutual conflict. When MipsHazardSchedule insert 'nop's,
+  // it potentially breaks some jumps, so they have to be expanded to long
+  // branches. When some branch is expanded to long branch, it potentially
+  // creates a hazard situation, which should be fixed by adding nops.
+  // This pass now combine these two passes, and runs them alternately until
----------------
Rework this comment to explain what the pass does rather than making references to MipsLongBranchPass and MipsHazardSchedule.


================
Comment at: test/CodeGen/Mips/branch-relaxation-with-hazard.ll:1-2
+; RUN: llc -O0 -mcpu=mips32r6 -relocation-model=pic < %s -o - | FileCheck %s --check-prefixes=CHECK,CHECK-PIC
+; RUN: llc -O0 -mcpu=mips32r6 -relocation-model=static < %s -o - | FileCheck %s --check-prefixes=CHECK-STATIC
+
----------------
Use -mtriple=mips-img-linux-gnu here and use the update_llc_checks.py script on this file.


================
Comment at: test/CodeGen/Mips/branch-relaxation-with-hazard.ll:4
+
+target triple = "mips-img-linux-gnu"
+
----------------
Remove this.


https://reviews.llvm.org/D46641