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

[Allen zhong via Phabricator via llvm-commits]

Allen marked 3 inline comments as done.
Allen 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">;
+
----------------
dmgreen wrote:
> Capitalize Schedule. Maybe reword as "Schedule vector stores by ascending address".
Thanks, apply your comment.


================
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);
 
----------------
dmgreen wrote:
> I think you can probably move these into AArch64InstrInfo, near to isPairedLdSt
Done, thanks!


================
Comment at: llvm/lib/Target/AArch64/AArch64MachineScheduler.cpp:29
+  case AArch64::STPQi:
+    return getLdStOffsetOp(*MI).getType() == MachineOperand::MO_Immediate;
+  }
----------------
dmgreen wrote:
> Why do we need to check it is an immediate?
this is because some IR used to match the **str	q0, [x8, :lo12:seed_lo] ** for example, and we can't get its offset





================
Comment at: llvm/lib/Target/AArch64/AArch64MachineScheduler.cpp:29
+  case AArch64::STPQi:
+    return getLdStOffsetOp(*MI).getType() == MachineOperand::MO_Immediate;
+  }
----------------
dmgreen wrote:
> Allen wrote:
> > dmgreen wrote:
> > > Why do we need to check it is an immediate?
> > this is because some IR used to match the **str	q0, [x8, :lo12:seed_lo] ** for example, and we can't get its offset
> > 
> > 
> > 
> Why is this needed?
if we don't check this, it will crash to use  **getImm()** get the  immediate with the above **str q0, [x8, :lo12:seed_lo] ** 


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

https://reviews.llvm.org/D126700