Page MenuHomePhabricator

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

Authored by JongwonLee on Apr 29 2016, 7:13 PM.

Details

Summary

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

Diff Detail

Event Timeline

JongwonLee updated this revision to Diff 55702.Apr 29 2016, 7:13 PM
JongwonLee retitled this revision from to [AArch64] Turn on "aarch64-ssa-load-store-opt" by default.
JongwonLee updated this object.
JongwonLee added reviewers: mcrosier, junbuml, jmolloy.
JongwonLee added a subscriber: llvm-commits.
rengolin requested changes to this revision.Apr 30 2016, 3:30 AM
rengolin added a reviewer: 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.

This revision now requires changes to proceed.Apr 30 2016, 3:30 AM
jmolloy edited edge metadata.Apr 30 2016, 10:25 AM
jmolloy added a subscriber: jmolloy.

Hi,

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

Cheers,

James

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?

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.

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

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.

JongwonLee abandoned this revision.May 4 2016, 12:12 AM