[AArch64] Turn on "aarch64-ssa-load-store-opt" by default

[JongwonLee]

Summary

This patch turns on "aarch64-ssa-load-store-opt" by default.

[rengolin]

This change request should be accompanied by benchmark numbers. If the one you shared in the other request are true, 10% compile time regression for zero execution time improvement in SPEC is unacceptable.

[jmolloy]

Hi,

Did the ssa patch get approved? I had outstanding questions on that that weren't resolved...

Cheers,

James

[JongwonLee]

In D19760#417741, @rengolin wrote:

This change request should be accompanied by benchmark numbers. If the one you shared in the other request are true, 10% compile time regression for zero execution time improvement in SPEC is unacceptable.

Hi,
I see the performance improvement for other commercial benchmark.
If some items does not lift up the performance for SPEC, those are unacceptable?

[JongwonLee]

In D19760#417818, @jmolloy wrote:

Hi,

Did the ssa patch get approved? I had outstanding questions on that that weren't resolved...

Cheers,

James

Hi,
The ssa patch is now under review.
I answered your questions in that patch.

[rengolin]

This is not just for SPEC. 10% build time regression in an important
benchmark with no performance improvement is a bad thing. The
improvements on the test-suite also show too small a margin to justify
such a big compiler-time regression.

If this was 2012, when Clang was 40~60% faster than GCC, I'd be ok
with the change once you can prove there's a relevant benchmark out
there that benefits immensely. So far, I haven't seen any evidence.

But right now, our compile time for most workloads have gone worse
than GCC, and 10% is a very big deal, especially now that James wants
to see the validity of this pass to other AArch64 cores.

Finally, such a big hit on compile time for a small back-end pass may
be an indication that you're doing something wrong. There may be a
much easier and faster way to do what you want, and I suggest you work
with James to make sure that situation improves.

cheers,
--renato

[JongwonLee]

In D19760#419206, @rengolin wrote:

This is not just for SPEC. 10% build time regression in an important
benchmark with no performance improvement is a bad thing. The
improvements on the test-suite also show too small a margin to justify
such a big compiler-time regression.

If this was 2012, when Clang was 40~60% faster than GCC, I'd be ok
with the change once you can prove there's a relevant benchmark out
there that benefits immensely. So far, I haven't seen any evidence.

But right now, our compile time for most workloads have gone worse
than GCC, and 10% is a very big deal, especially now that James wants
to see the validity of this pass to other AArch64 cores.

Finally, such a big hit on compile time for a small back-end pass may
be an indication that you're doing something wrong. There may be a
much easier and faster way to do what you want, and I suggest you work
with James to make sure that situation improves.

cheers,
--renato

Thank you for the description.

lib/Target/AArch64/AArch64TargetMachine.cpp

static cl::opt<bool>
     EnableLoopDataPrefetch("aarch64-loop-data-prefetch", cl::Hidden,
                            cl::desc("Enable the loop data prefetch pass"),
                            cl::init(true));
 
 static cl::opt<bool>
     EnableSSALoadStoreOpt("aarch64-ssa-load-store-opt",
                           cl::desc("Enable the load/store pair"
                                    " optimization pass in SSA form"),
-                          cl::init(false), cl::Hidden);
+                          cl::init(true), cl::Hidden);
 
 extern "C" void LLVMInitializeAArch64Target() {
   // Register the target.
   RegisterTargetMachine<AArch64leTargetMachine> X(TheAArch64leTarget);
   RegisterTargetMachine<AArch64beTargetMachine> Y(TheAArch64beTarget);
   RegisterTargetMachine<AArch64leTargetMachine> Z(TheARM64Target);
   auto PR = PassRegistry::getPassRegistry();
   initializeGlobalISel(*PR);

test/CodeGen/AArch64/ssa-ldst-opt.ll

-; RUN: llc -march=aarch64 -verify-machineinstrs -asm-verbose=false -aarch64-ssa-load-store-opt -o - %s | FileCheck %s
+; RUN: llc -march=aarch64 -verify-machineinstrs -asm-verbose=false -o - %s | FileCheck %s
 
 ; This test is for 'AArch64 load/store optimization in SSA form'.
 ; Find consecutive two 32-bit loads and consecutive two 32-bit stores that write the values of the consecutive 32-bit loads.
 ; Transform the loads/stores to 64-bit load/store.
 
 ; CHECK-LABEL: test_offset_no_changed:
 ; CHECK:        ldur x8, [x0, #-76]
 ; CHECK-NEXT:   stur x8, [x1, #-44]