[PATCH] D37698: Add target callback doMemOpsHaveSameBasePtr

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: lib/Target/AMDGPU/SIInstrInfo.cpp:360
 
+bool SIInstrInfo::doMemOpsHaveSameBasePtr(const MachineInstr &MI1,
+                                          unsigned BaseReg1,
----------------
hfinkel wrote:
> Can you make *this* the default implementation?
This will result in both improvements an regressions across the board as it will increase amount of clustering dramatically. Having no access to all HW and benchmark suites and targets I left the default behavior intact. In particular I had to limit clustering in amdgpu below to maintain reasonable average performance, which I cannot do for all other targets.

I.e. I would be more than happy to  make it default behavior, but I think much more people should agree to spend their time tuning it after for their targets.

What we can do, if this is eventually clonned to the majority of targets requiring clustering, then change the default.

What others think? Is this a generally desired default behaviour?


https://reviews.llvm.org/D37698