[PATCH] D86864: [MachineSinking] sink more profitable loads

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet accepted this revision.
qcolombet added a comment.
This revision is now accepted and ready to land.

> I guess the compiling time increase on these benchmarks should be acceptable?

Yes, that's fine by me.



================
Comment at: llvm/lib/CodeGen/MachineSink.cpp:348
     ToSplit.clear();
+    StoreInstrCache.clear();
+    HasStoreCache.clear();
----------------
shchenz wrote:
> qcolombet wrote:
> > Would it make sense to preserve the cache as long as we don't move stores, calls and loads with ordered memory ref?
> > 
> > I'm worried about the compile time impact of this patch so I am wondering if we want to be smarter about invalidating the cache.
> Good idea. But seems we still need to clear the cache before we return from `MachineSinking::runOnMachineFunction`. (`MachineSink` instance is shared with different callings to `MachineSinking::runOnMachineFunction` for different functions), otherwise I meet some weird memory issue.
Yeah, that's expected.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86864