[AArch64] Disable narrow load merge by default

[junbuml]

Summary

As this optimization converts two loads into one load with two shift instructions,
it could potentially hurt performance if a loop is arithmetic operation intensive.

[mcrosier]

Based on our results and feedback from our SD colleagues, I'm fine with approving this patch. I know the performance results were neutral for Spec2006. Did you do any additional testing on Spec2000 or EEMBC by chance?

[mcrosier]

Approving, but feel free to wait for feedback from Tim, James, or others before committing.

[junbuml]

No performance regression was found in spec200/2006. I will run EEMBC as well.

[jmolloy]

Hi Jun,

Have you considered deciding this as a MachineCombiner pattern? This would be a good place to know if the loop is arithmetic or load/store heavy.

Cheers,

James

[junbuml]

Have you considered deciding this as a MachineCombiner pattern? This would be a good place to know if the loop is arithmetic or load/store heavy.

Let me take a look if we can move this to MachineCombiner.
Thanks James!

[evandro]

WRT Exynos M1, this change is neutral.

LGTM

[junbuml]

I had multiple EEMBC runs as my score were somewhat unstable. Overall, I wasn't able to see reproducible regressions. Please feel free to run performance tests and share your results. I will commit this at the end of this week if there is no objection.

[mcrosier]

In D20172#433201, @junbuml wrote:

I will commit this at the end of this week if there is no objection.

No objection here.

[junbuml]

Have you considered deciding this as a MachineCombiner pattern? This would be a good place to know if the loop is arithmetic or load/store heavy.

I think MachineCombiner is also good place to perform this optimization with minor changes in the profitability check. As of now, however, I don't have any case impacted by this optimization. So, I will deprioritize doing it until I can find the cases.

lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp

static cl::opt<unsigned> LdStLimit("aarch64-load-store-scan-limit",
                                    cl::init(20), cl::Hidden);
 
 // The UpdateLimit limits how far we search for update instructions when we form
 // pre-/post-index instructions.
 static cl::opt<unsigned> UpdateLimit("aarch64-update-scan-limit", cl::init(100),
                                      cl::Hidden);
 
 static cl::opt<bool> EnableNarrowLdMerge("enable-narrow-ld-merge", cl::Hidden,
-                                         cl::init(true),
+                                         cl::init(false),
                                          cl::desc("Enable narrow load merge"));
 
 namespace llvm {
 void initializeAArch64LoadStoreOptPass(PassRegistry &);
 }
 
 #define AARCH64_LOAD_STORE_OPT_NAME "AArch64 load / store optimization pass"
 

test/CodeGen/AArch64/arm64-narrow-ldst-merge.ll

-; RUN: llc < %s -mtriple aarch64--none-eabi -mcpu=cortex-a57 -verify-machineinstrs | FileCheck %s --check-prefix=CHECK --check-prefix=LE
-; RUN: llc < %s -mtriple aarch64_be--none-eabi -mcpu=cortex-a57 -verify-machineinstrs | FileCheck %s --check-prefix=CHECK --check-prefix=BE
-; RUN: llc < %s -mtriple aarch64--none-eabi -mcpu=kryo -verify-machineinstrs | FileCheck %s --check-prefix=CHECK --check-prefix=LE
+; RUN: llc < %s -mtriple aarch64--none-eabi -mcpu=cortex-a57 -verify-machineinstrs -enable-narrow-ld-merge=true | FileCheck %s --check-prefix=CHECK --check-prefix=LE
+; RUN: llc < %s -mtriple aarch64_be--none-eabi -mcpu=cortex-a57 -verify-machineinstrs -enable-narrow-ld-merge=true | FileCheck %s --check-prefix=CHECK --check-prefix=BE
+; RUN: llc < %s -mtriple aarch64--none-eabi -mcpu=kryo -verify-machineinstrs -enable-narrow-ld-merge=true | FileCheck %s --check-prefix=CHECK --check-prefix=LE
 
 ; CHECK-LABEL: Ldrh_merge
 ; CHECK-NOT: ldrh
 ; CHECK: ldr [[NEW_DEST:w[0-9]+]]
 ; CHECK-DAG: and [[LO_PART:w[0-9]+]], [[NEW_DEST]], #0xffff
 ; CHECK-DAG: lsr [[HI_PART:w[0-9]+]], [[NEW_DEST]], #16
 ; LE: sub {{w[0-9]+}}, [[LO_PART]], [[HI_PART]]
 ; BE: sub {{w[0-9]+}}, [[HI_PART]], [[LO_PART]]