[PATCH] D149768: [ArgumentPromotion] Bail if any callers are minsize

Fri May 5 10:56:25 PDT 2023

aeubanks added a comment.

In D149768#4322662 <https://reviews.llvm.org/D149768#4322662>, @smeenai wrote:

> In D149768#4320982 <https://reviews.llvm.org/D149768#4320982>, @aeubanks wrote:
>
>> but another thing to consider is that we only recently started running argpromo for -O1/2/s/z. I can see two cases where this patch would regress -Oz code size. one is running an -O3 post-link with an -Oz pre-link, which wouldn't make sense in general. the second is that https://reviews.llvm.org/D148269 actually helped with -Oz code size in your case, then this patch regressed it back to where it was before
>
> I was thinking about this too. I verified that this patch is what caused the regression, and then the reland of D148269 <https://reviews.llvm.org/D148269> (which took place after this patch) had no effect for us.
>
> How does that pipeline change work for FullLTO though? Does the `Level == OptimizationLevel::O3` check only apply to the pre-link compilations, or does it apply to the actual FullLTO phase based on the `--lto-Ox` value as well? We should just be using the default for LTO, which is `--lto-O2` IIRC. We do build some TUs with `-O3` and some TUs with `-Oz` that are FullLTO'd together.
>
> I'm trying to think of a way to provide a reduced example demonstrating the size increase, but it's proving to be pretty tricky. This is a pretty large FullLTO build from a mix of `-Oz` and `-O3` resources, and there seem to be interactions with other optimizations (in particular outlining) that are significant, so it'll take me a while to narrow things down.

Ah you're using FullLTO, which always runs ArgPromo on the merged module for all opt levels, that makes sense then. I'm fine with reverting these two patches while you investigate. I'm not in a rush to get these landed, but I do think that some version of this patch is valuable for code size even on its own.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149768/new/

https://reviews.llvm.org/D149768