[PATCH] D18048: [AArch64] Enable more load clustering in the MI Scheduler.

[Geoff Berry via llvm-commits]

gberry added inline comments.

================
Comment at: lib/Target/AArch64/AArch64InstrInfo.cpp:1502
@@ +1501,3 @@
+    return false;
+  case AArch64::LDRWui:
+  case AArch64::LDURWi:
----------------
mcrosier wrote:
> gberry wrote:
> > Do we always merge zext with sext loads?  I would think this would be controlled by the subtarget check we do for combining narrow loads since this combination has the same trade off with load vs arith and increased depency chain?
> Yes; these are unconditionally merged in the load/store optimization pass.
Okay.  It seems like we may want to reconsider this.

================
Comment at: lib/Target/AArch64/AArch64InstrInfo.cpp:1519
@@ -1460,3 +1518,3 @@
                                           unsigned NumLoads) const {
   // Only cluster up to a single pair.
   if (NumLoads > 1)
----------------
mcrosier wrote:
> gberry wrote:
> > Shouldn't we be checking isCandidateToMergeOrPair here to avoid clustering loads that we're not going to pair?
> This is the purpose of the canPairLdStOpc() check.  It's effectively the pairing logic from the load/store optimizer isCandidateToMergeOrPair() function, but without the merging logic.
But canPairLdStOpc() is only considering the opcode.  It doesn't take into account the other things that may inhibit pairing later such as isLdStSuppressed being true.  You may be asking the scheduler to cluster loads that you're not going to end up pariing later.


http://reviews.llvm.org/D18048