[PATCH] D14183: [AArch64]Extend merging narrow loads into a wider load

[Chad Rosier via llvm-commits]

mcrosier added a comment.

Jun,
You might consider breaking this up into two separate patches:

1. Add the enableNarrowLdPromotion() functionality to address James' concern about this change not necessarily being profitable on all targets.
2. Add the functional change for merging additional narrow loads into wider loads.

  Chad


================
Comment at: lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp:88
@@ -86,3 +87,3 @@
   const TargetRegisterInfo *TRI;
-  bool IsStrictAlign;
+  bool IsNarrowLdOptProfitable;
 
----------------
I don't think you need a member variable.  See my comment below.

================
Comment at: lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp:1242
@@ -1166,3 +1241,3 @@
   // Three tranformations to do here:
-  // 1) Find halfword loads that can be merged into a single 32-bit word load
+  // 1) Find narrow loads that can be converted into a single wider load
   //    with bitfield extract instructions.
----------------
Please add a period.

================
Comment at: lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp:1267
@@ -1191,3 +1266,3 @@
   for (MachineBasicBlock::iterator MBBI = MBB.begin(), E = MBB.end();
-       !IsStrictAlign && MBBI != E;) {
+       IsNarrowLdOptProfitable && MBBI != E;) {
     MachineInstr *MI = MBBI;
----------------
Why not have enableNarrowLdPromotion(MBB->getParent()) return a bool and call it here directly, rather than store in a member variable?  AFAICT, this is the only use.


http://reviews.llvm.org/D14183