[SR-9288] Unswitch path annotation or builtin

[177d8476-2756-4152-91d7-984f74d3896c]

Previous ID	SR-9288
Radar	rdar://problem/52194058
Original Reporter	@milseman
Type	Bug

Additional Detail from JIRA

| | | |------------------|-----------------| |Votes | 0 | |Component/s | Compiler | |Labels | Bug, Optimizer | |Assignee | @eeckstein | |Priority | Medium | md5: 833f461a11be670dc9fa0a9467419cf9

Issue Description:

Swift's iterator design means that we are not in control of the shape of user loops over our types. String's iterators would greatly benefit from a targeted form of loop-unswitching for the common fast path, and LLVM branch-weight metadata alone isn't cutting it for us here. See https://bugs.swift.org/browse/SR-9287

I do not believe in asking for aggressive loop unswitching or even LICM, which is often a pessimization, but there are certain cases where this would be beneficial. We need some mechanism to communicate what path of an iteration loop should be unswitched. Perhaps something analogous to `_fastPath` e.g. `_loopUnswitchedPath`.

[177d8476-2756-4152-91d7-984f74d3896c]

@swift-ci create

[belkadan]

Boo. -1 on features that claim to know the code better than the compiler.

[177d8476-2756-4152-91d7-984f74d3896c]

I'm sorry, but I know the code better than the compiler. There is no magic analysis that will change that fact. What is your alternative to things like __builtin_expect?

[belkadan]

"Please unswitch me" is an implementation decision. The thing I'd want is extra semantic information, for example "this mutating function will not change the active case of the enum".

[177d8476-2756-4152-91d7-984f74d3896c]

" -1 on features that claim to know the code better than the compiler." is a statement of principle, and I'm trying to understand it. What is your alternative to __builtin_expect, which is an explicit statement from the programmer that they know the code better than the compiler.

[belkadan]

Sorry, I guess I phrased it badly. "Features that claim to know how to compile the code better than the compiler" is probably more what I meant. __builtinexpect / _fastPath is a statement about semantics that the base language doesn't have a way to express. _loopUnswitchedPath is more like inline(\_always).

[177d8476-2756-4152-91d7-984f74d3896c]

I see, so you're in favor of specifying semantics but not in favor of explicit compile-it-this-way annotations. That's understandable from a general user model, but the only reason I'm using such semantic annotations is to affect compilation in a very specific way.

__builtin_expect as a semantic annotation is nice, it has the advantage of potentially affecting multiple compilation decisions, e.g. both inlining and code layout, so I'm certainly not opposed to semantic annotations. However, unless there's strong, enforced guaranteed optimizations, semantic annotations carry another level of indirection in reasoning about performance. In the good cases, where a performance issue is spotted immediately, it can result in many late nights staring at disassembly trying to massage source to produce efficient output. In the bad cases, we never even detect it.

I'm fine either with a semantic model with guaranteed optimizations, or a low-level compile-it-this-way. How would you design a semantic model which results in being able to accomplish something like having a `Builtin.unswitchThisPath()`.

[belkadan]

I'm against having that capability. You're adding it based on today's performance testing; it will take another round of performance testing to take it out should the tradeoffs ever change. If a condition is consistent through repetitions of a loop, it should be possible to say that and the compiler should know what to do with that information.

[Catfish-Man]

I ran into something like this today where I have a loop with two loop-invariant flags, so I can either have 4 loops with a good bit of repeated code, or one loop with two conditionals I'd like unswitched.

The latter is significantly shorter and easier to read, but unfortunately with today's swiftc results in significantly slower code. Admittedly, some of that may simply be that the ARC optimizer was unable to figure out that it could remove a retain-release pair under certain conditions, so unswitching might indeed fade into the noise as that part improves.