[PATCH] D89447: [MachineInstr] Add support for instructions with multiple memory operands.

[Michael Liao via Phabricator via llvm-commits]

hliao added a comment.

In D89447#2336699 <https://reviews.llvm.org/D89447#2336699>, @jonpa wrote:

>> I removed that check that one of those two MMOs needs to be store. No change to test now. But, that sounds a little weird that one instruction claims mayLoad and mayStore but only has isLoad MMO. Maybe we need to change that MMO in prefetch to be both isLoad and isStore.
>
> The SystemZ Prefetch Data (PFD) instruction has a flag bit to control if the prefetch is read or write, so in a way those two flags make sense. However, since the PFD does not in fact clobber any memory I think the idea behind the mayLoad and mayStore flags is to keep the instruction in place in the block as much as possible. It shouldn't necessarily matter, but generally it is probably better spread out the memory accesses / prefetches rather than having them all happen at once, I would think. If the block is big with many prefetches they shouldn't all end up in the end of the block...
>
> If you think that check is valuable, then maybe we could try to find another way to keep the PFD instructions in their places. I tried adding hasSideEffects = 1 on PFD/PFDRL instructions instead of removing the check on the MMOs, but that did not seem to be NFC on benchmarks...

Not only target-specific PREFETCH has that, the target-independent PREFETCH has both mayLoad and mayStore. I thought that maybe added in the early day to hour the program order. Even if a prefetch may prefetch for write or precisely for the ownership, it really didn't change that memory at all. The order of PREFETCH needs to be honored by the scheduler is due to it don't understand the target timing yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89447