[PATCH] D54847: [MachineScheduler] Add support for clustering mem ops with FI base operands

Tue Nov 27 07:33:16 PST 2018

niravd added inline comments.

================
Comment at: lib/Target/AArch64/AArch64InstrInfo.cpp:1159
+        (BaseOpA->isFI() && BaseOpB->isFI() &&
+         BaseOpA->getIndex() == BaseOpB->getIndex())) {
       int LowOffset = OffsetA < OffsetB ? OffsetA : OffsetB;
----------------
thegameg wrote:
> niravd wrote:
> > You need to do slightly more checking here as there may be operations that cross Frame index boundaries. SelectionDAGAddressAnalysis has a relevant example.
> Please correct me if I'm wrong, but from what I see in `SelectionDAGAddressAnalysis`, it tries to match **non-equal** FIs. Are you suggesting that we should also support this here? Or that comparing the FIs is not enough?
Right. Comparing FIs is not enough because for accesses based on fixed object frames indices may operate on with memory in another fixed object frame index.

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

https://reviews.llvm.org/D54847