[PATCH] D136806: [Pipelines] Introduce SROA after (final, full) loop unrolling

dyung added a comment. Hi @lebedev.ri, we recently noticed a failure in one of our internal tests which I bisected back to your change. I have filed the details in Issue #59122 . Can you take a look? Repository: rG LLVM Github Monorepo CHAN

[PATCH] D136806: [Pipelines] Introduce SROA after (final, full) loop unrolling

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8adfa29706e5: [Pipelines] Introduce SROA after (final, run-time) loop unrolling (authored by lebedev.ri). Repository: rG LLVM Github Monorepo CHA

[PATCH] D136806: [Pipelines] Introduce SROA after (final, full) loop unrolling

lebedev.ri added a comment. In D136806#3934445 , @asbirlea wrote: > Green light perf-wise. > I cannot comment on whether the position is "the right" one though. I'm > deferring to the other reviewers. Thank you! Repository: rG LLVM Github Monorepo

[PATCH] D136806: [Pipelines] Introduce SROA after (final, full) loop unrolling

asbirlea added a comment. Green light perf-wise. I cannot comment on whether the position is "the right" one though. I'm deferring to the other reviewers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136806/new/ https://reviews.llvm.org/D136806

[PATCH] D136806: [Pipelines] Introduce SROA after (final, full) loop unrolling

lebedev.ri added a comment. In D136806#3932369 , @asbirlea wrote: > As far as performance, this looks fine. Not seeing measurable gains. Thank you for checking! Is the evaluation still ongoing, or is this the green light to go ahead? Repository: rG L

[PATCH] D136806: [Pipelines] Introduce SROA after (final, full) loop unrolling

asbirlea added a comment. As far as performance, this looks fine. Not seeing measurable gains. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136806/new/ https://reviews.llvm.org/D136806 ___ cfe-commits m

[PATCH] D136806: [Pipelines] Introduce SROA after (final, full) loop unrolling

lebedev.ri updated this revision to Diff 475925. lebedev.ri marked an inline comment as done. lebedev.ri added a comment. Herald added a subscriber: pcwang-thead. Adjust full LTO pipeline, for symmetry with non-full LTO pipeline. Looks like there is test coverage shortage. Repository: rG LLVM

[PATCH] D136806: [Pipelines] Introduce SROA after (final, full) loop unrolling

lebedev.ri added a comment. In D136806#3931860 , @spatel wrote: > LGTM based on the timing, results, and alternatives discussed Thank you for the review. > There's some testing in progress according to previous comments, so best to > wait for that to f

[PATCH] D136806: [Pipelines] Introduce SROA after (final, full) loop unrolling

spatel accepted this revision. spatel added a comment. This revision is now accepted and ready to land. LGTM based on the timing, results, and alternatives discussed There's some testing in progress according to previous comments, so best to wait for that to finish in case it turns up anything ne

[PATCH] D136806: [Pipelines] Introduce SROA after (final, full) loop unrolling

lebedev.ri added a comment. In D136806#3928663 , @asbirlea wrote: > IIUC compile time impact for adding another SROA (the one outside LTO) is > negligible? Yup. > Regarding the principle of adding another pass and where in the pipeline, > we're still

[PATCH] D136806: [Pipelines] Introduce SROA after (final, full) loop unrolling

asbirlea added a comment. IIUC compile time impact for adding another SROA (the one outside LTO) is negligible? Regarding the principle of adding another pass and where in the pipeline, we're still at a case by case basis. We had a discussion/round table at LLVM Dev on the documenting the curr

[PATCH] D136806: [Pipelines] Introduce SROA after (final, full) loop unrolling

lebedev.ri added a comment. In D136806#3926319 , @arsenm wrote: > I think SROA after unroll is important. It's practically the main reason to > unroll Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:1200-1201 FPM.addPass(W

[PATCH] D136806: [Pipelines] Introduce SROA after (final, full) loop unrolling

lebedev.ri updated this revision to Diff 475311. lebedev.ri retitled this revision from "[Pipelines] Introduce SROA after (full) loop unrolling" to "[Pipelines] Introduce SROA after (final, full) loop unrolling". lebedev.ri added a reviewer: arsenm. lebedev.ri added a comment. Herald added subscr