[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

This revision was automatically updated to reflect the committed changes. Closed by commit rG03bd5198b6f7: [OldPM] Pass manager: run SROA after (simple) loop unrolling (authored by lebedev.ri). Changed prior to commit: https://reviews.llvm.org/D87972?vs=296005&id=296028#toc Repository: rG LL

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

lebedev.ri added a comment. In D87972#2310603 , @xbolva00 wrote: >> ! In D87972#2310595 , @nikic wrote: >> >>> I'll just say this LGTM as it establishes parity with what NewPM has been >>> doing for a while already

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

xbolva00 accepted this revision. xbolva00 added a comment. >> I'll just say this LGTM as it establishes parity with what NewPM has been >> doing for a while already. +1 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87972/new/ https://reviews.llvm

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. I'll just say this LGTM as it establishes parity with what NewPM has been doing for a while already. Reviewers, in the future, please reject any patches that only change the NewPM pipeline or o

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

lebedev.ri updated this revision to Diff 296005. lebedev.ri added a comment. Re-fix clang tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87972/new/ https://reviews.llvm.org/D87972 Files: clang/test/CodeGenCXX/union-tbaa2.cpp clang/test/Mi

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

xbolva00 added a comment. >> I'm not really sure what are my potential next steps here. Maybe just add option to disable late SROA? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87972/new/ https://reviews.llvm.org/D87972 _

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

lebedev.ri added a comment. In D87972#2294614 , @lebedev.ri wrote: > In D87972#2294488 , @xbolva00 wrote: > Does that sound reasonable? >> >> Yes IMHO. >> What are the next suggested steps? >> >> It would

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

lebedev.ri updated this revision to Diff 295817. lebedev.ri added a comment. Herald added a subscriber: pengfei. Rebased, NFC Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87972/new/ https://reviews.llvm.org/D87972 Files: llvm/lib/Target/AMDGPU/

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

lebedev.ri added a comment. In D87972#2294488 , @xbolva00 wrote: >>> Does that sound reasonable? > > Yes IMHO. > >>> What are the next suggested steps? > > It would be great to isolate and check the cases which regressed a bit. I've rerun my benchmark, an

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

xbolva00 added a comment. >> Does that sound reasonable? Yes IMHO. >> What are the next suggested steps? It would be great to isolate and check the cases which regressed a bit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87972/new/ https://rev

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

lebedev.ri added a comment. @MaskRay, @dmgreen & @sanwou01 thank you for running perf experiment! I think all the results are consistent along the lines of "this sounds generally reasonable (esp. given that new-pm does it already), as usual results in ups&downs, but seems to be a (small) geomean

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

sanwou01 added a comment. SPEC 2017 on AArch64 is neutral on the geomean. The only slight worry is omnetpp with a 1% regression, but this is balanced by a .8% improvement on mcf. Other changes are in the noise. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

dmgreen added a comment. > @dmgreen for arm?:) This would seem more like a good general codegen cleanup than something that would be target dependent. It would probably be more dependent on the code that is being run, than the exact target. But yeah, I ran some baremetal tests. Only one change

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

fhahn added a comment. In D87972#2284060 , @MaskRay wrote: > I have tested this patch internally and seen gains and losses. On one > document search related benchmark 3~5% improvement. One zippy (snappy) there > is 3~5% regression. Perhaps we do need a c

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

xbolva00 added a comment. X86 data collected by @lebedev.ri looks good as well. @greend for arm?:) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87972/new/ https://reviews.llvm.org/D87972 ___ cfe-commit

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

rampitec added a comment. This is obviously LGTM from the AMDGPU BE point of view, we did it ourselves. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87972/new/ https://reviews.llvm.org/D87972 ___ cfe-co

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

hliao added a comment. In D87972#2284096 , @lebedev.ri wrote: > In D87972#2284064 , @xbolva00 wrote: > >> In D87972#2284060 , @MaskRay wrote: >> >>> I have tested this patch

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

xbolva00 added inline comments. Comment at: clang/test/Misc/loop-opt-setup.c:2 +// RUN: %clang -O1 -fexperimental-new-pass-manager -fno-unroll-loops -S -o - %s -emit-llvm | FileCheck %s -check-prefixes=CHECK-ALL,CHECK-NEWPM +// RUN: %clang -O1 -fno-experimental-new-pass-manager

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

lebedev.ri added a comment. In D87972#2285176 , @arsenm wrote: > I assume this makes 1f4e7463b5e3ff654c84371527767830e51db10d > > redundant? Yes, see `llvm/lib/Target/AMDGPU/AMDGPUTar

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

arsenm added a comment. I assume this makes 1f4e7463b5e3ff654c84371527767830e51db10d redundant? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87972/new/ https://reviews.llvm.org

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

lebedev.ri added a comment. In D87972#2284064 , @xbolva00 wrote: > In D87972#2284060 , @MaskRay wrote: > >> I have tested this patch internally and seen gains and losses. On one >> document search related benchmark

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

xbolva00 added a comment. In D87972#2284060 , @MaskRay wrote: > I have tested this patch internally and seen gains and losses. On one > document search related benchmark 3~5% improvement. One zippy (snappy) there > is 3~5% regression. Perhaps we do need

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

lebedev.ri added a comment. (I'm guessing that we are talking about run-time performance here.) In D87972#2284060 , @MaskRay wrote: > I have tested this patch internally and seen gains and losses. On one > document search related benchmark 3~5% improveme

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

xbolva00 added a comment. In D87972#2284060 , @MaskRay wrote: > I have tested this patch internally and seen gains and losses. On one > document search related benchmark 3~5% improvement. One zippy (snappy) there > is 3~5% regression. Perhaps we do need

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

MaskRay added a comment. I have tested this patch internally and seen gains and losses. On one document search related benchmark 3~5% improvement. One zippy (snappy) there is 3~5% regression. Perhaps we do need a conditional extra SROA run. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

xbolva00 added a comment. https://reviews.llvm.org/D68593 added late SROA to NPM so it would be good to enable it for LPM as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87972/new/ https://reviews.llvm.org/D87972 __

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

fhahn added a comment. Surprising this causes only such a small perf regression. I guess it should be OK given that, but here probably are some pathological cases out there where this may cause some noticeable compile-time regressions. IIUC the additional cases this catches come mainly from ful

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

lebedev.ri updated this revision to Diff 292984. lebedev.ri added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fixing a few clang tests and updating one more llvm test to check this also. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http