[stdlib] Refactor min()/max()

[PatrickPijnappel]

Refactors min() and max():

• Adds a comment about [stdlib] Make comparison operator choices consistent #137.
• Makes min() consistent with max() changes introduced in [tests] basic min/max test cases #566.
• Removes unnecessary local variable abbreviations.
• Passes build-script -R -t

Important
Comparing the generated SIL, the new version seems to be worse. Also, the calls to min(_:_:) from min(_:_:_:_:) do not appear to be inlined. I'm not sure if this wil still be optimized away, or that I'm invoking the sil generation wrong? I'm using $ swiftc -O -emit-sil New.swift -o New.sil.c. If this version is indeed less performant, that would also apply to the changes to max() introduced in #566.

[gribozavr]

I think we need to make sure that those calls get inlined.

@nadavrot What do you think about marking the two-argument min() and max() with @inline(__always)?

@PatrickPijnappel Could you add a clarification to the documentation comment that explains the new guarantee? Something like this (better wording would be appreciated!) :

/// When the sequence contains multiple equal least values, returns the first one.
func min(...) { ... }

[nadavrot]

@gribozavr The inliner should know when to inline these functions.

The patch LGTM (assuming there are no perf regressions).

[PatrickPijnappel]

@nadavrot Did you see the note about the generated SIL in the description? The SIL appears worse, and the two-argument min() and max() functions do not appear to be inlined. Unless I'm not generating the SIL correctly (see description), should I file a bug?

@gribozavr Done. Couldn't much improve on the wording unfortunately.

[gribozavr]

LGTM, but waiting for a reply from @nadavrot.

[nadavrot]

@gribozavr We don't inline generic code. The optimization strategy we use is to first specialize generics, and then clean things up (arc, load/store optimizations, etc). There is not much we can do with generic code because we don't know anything about the type T.

If you are concerned about the performance of this code then I suggest writing a simple benchmark with integers. Something like max-ing all of the integers in an array or something. This is a typical usecase and it will allow the compiler to kick-in.

I am not concerned about this change and expect to see no changes in performance.

[gribozavr]

@PatrickPijnappel Could you do a simple benchmark like @nadavrot advises, just to be safe?

[PatrickPijnappel]

@gribozavr @nadavrot Just ran a small benchmark, performance (and SIL even) is indeed identical in actual usage.

[gribozavr]

@PatrickPijnappel Thank you for verifying!