Performance regression with niche optimization

[mikebenfield]

After PR #94075 for more niche optimizations was merged, there were some performance regressions.

Some performance regressions are possibly just due to the extra arithmetic and branch or cmov required when getting a discriminant out of a tag. But notably when compiling syn, the regression is largely due to extra time in LLVM_lto_optimize.

It would be nice to understand this better, and ideally do something about it.

[Rageking8]

@rustbot label +T-compiler +regression-untriaged

[nnethercote]

#102035 is a related example, where increased use of niche-filling causes some small but widespread instruction count regressions.

[apiraino]

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-medium

[mikebenfield]

A little more data:

I did some local testing using the "Self profile" mode of the perf tool, compiling syn for the Opt profile and scenario IncrPatched (the same profile at the link above). I compared a recent master to a modifed master turning off niche optimizations when more than one variant has data.

The results I get locally make a lot more sense to me. The linked data shows the LLVM_lto_optimize step taking 42% of total time, which seems odd. Locally it takes only around 0.1% of total time. Instead, a lot of time is taken in various particular LLVM passes, which also seems to be where all/most of the increase comes in with more niche optimizations. Notably InstCombinePass.

[mikebenfield]

In addition to #102872, I have in mind two other optimizations for switching on a discriminant:

First, rather than this llvmir-like pseudocode:

discr =  select is_untagged, untagged_discr, tag
select disc [
    ....
]

we could instead do this:

if is_untagged { jump straight to untagged basic block }
select tag [
   ...
]

Replacing a conditional move with a jump. I am fairly certain this would be a win, as it would allow to skip the tag calculations whenever we have the untagged variant, it would let us remove the cmov, which has data dependencies on a lot of the other instructions, and the new jump is at least as predictable as the existing one.

I don't think it's possible to make this happen just by modifying the current codegen_get_discr. I think there are 3 possibilities:

1. introduce a MIR pass to do this (but it would need to happen after monomorphization)
2. introduce a new TerminatorKind::SwitchDiscr, and then make this happen during codegen
3. do it in LLVM during some pass

The second optimization I have in mind would need to happen in LLVM.

[mikebenfield]

I filed this LLVM issue, which if addressed would lead to better code in niche match statements in many cases.

[apiraino]

@mikebenfield did #102872 solved this issue? Checking progress to unblock #102035. Thanks!

[mikebenfield]

@apiraino Honestly I don’t know. It definitely improved some things, but it’s not clear to be which benchmarks are particularly relevant, so I’m not sure whether we can say this issue is solved.