[PATCH] D126700: [MachineScheduler] Order more stores by ascending address

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64.td:219
+def FeatureAscendStoreAddress : SubtargetFeature<"ascend-store-address",
+    "IsStoreAddressAscend", "false", "schedule to ascend the address of stores">;
+
----------------
Capitalize Schedule. Maybe reword as "Schedule vector stores by ascending address".


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.h:106
 
+  static bool isPairedLdSt(const MachineInstr &MI);
+
----------------
Can you add a /// doc string


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.h:497-498
 unsigned getBLRCallOpcode(const MachineFunction &MF);
+const MachineOperand &getLdStBaseOp(const MachineInstr &MI);
+const MachineOperand &getLdStOffsetOp(const MachineInstr &MI);
 
----------------
I think you can probably move these into AArch64InstrInfo, near to isPairedLdSt


================
Comment at: llvm/lib/Target/AArch64/AArch64MachineScheduler.cpp:29
+  case AArch64::STPQi:
+    return getLdStOffsetOp(*MI).getType() == MachineOperand::MO_Immediate;
+  }
----------------
Why do we need to check it is an immediate?


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

https://reviews.llvm.org/D126700