Enable fast floating point model and fast debug linking by lhecker · Pull Request #11466 · microsoft/terminal

lhecker · 2021-10-08T20:41:28Z

This commit enables /fp:fast. This doubles the performance of the Delta E
computation in #11095 for instance. Additionally it re-enables two options for
debug builds which are normally enabled by default by Visual Studio.

PR Checklist

I work here
Tests added/passed

Validation Steps Performed

No change in binary size
No obvious change in behavior

DHowett

Love it

DHowett · 2021-10-08T20:53:17Z

I believe we should ship this with 1.12 so dE doesn't regress launch perf as significantly 😄

(how much speed are we losing, would you say, on startup right now?)

DHowett · 2021-10-08T21:37:16Z

@msftbot merge this in 5 minutes

ghost · 2021-10-08T21:37:19Z

Hello @DHowett!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

I won't merge this pull request until after the UTC date Fri, 08 Oct 2021 21:42:18 GMT, which is in 5 minutes

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

Rosefield · 2021-10-08T21:38:36Z

curious if this has any effect on directional movement if you have a bunch of panes open. It should be fine since I'm checking for an epsilon of 1e-4, but who knows how bad accuracy could get.

lhecker · 2021-10-08T21:43:04Z

The precise model isn't actually really that much more precise than the fast one...
It's precise because it handles all the corner cases (NaN stuff) while the fast one doesn't.
You can read more about it here: https://devblogs.microsoft.com/cppblog/do-you-prefer-fast-or-precise/

DHowett · 2021-10-08T21:55:23Z

build's failing, curiously!

lhecker · 2021-10-08T22:29:09Z

The tests work on my AMD CPU. The CI servers are clearly running on Intel CPUs. heh. 😎
In all seriousness however it a) sucks because I can't debug it and b) I'm surprised that TAEF doesn't compare floats using a sensible epsilon.

lhecker · 2021-10-09T01:12:03Z

ez

lhecker · 2021-10-09T01:13:51Z

@msftbot merge this in 5 minutes

ghost · 2021-10-09T01:13:54Z

Hello @lhecker!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

I won't merge this pull request until after the UTC date Sat, 09 Oct 2021 01:18:54 GMT, which is in 5 minutes

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

DHowett

Blocking to review the new!

src/cascadia/ut_app/ColorHelperTests.cpp

DHowett · 2021-10-09T01:16:39Z

src/inc/consoletaeftemplates.hpp

+        }
+
+        const auto nDiff = static_cast<std::make_signed_t<U>>(*reinterpret_cast<U*>(&a) - *reinterpret_cast<U*>(&b));
+        const auto uDiff = static_cast<U>(nDiff < 0 ? -nDiff : nDiff);


What the heck? This is legal??

And not UB?

No this is 100% undefined behavior. But it works because MSVC doesn't do strict aliasing. In C++20 you can use std::bit_cast.

Regarding the question about legality of reading floats like that: Yeah that's legal, because the bit patterns of IEEE floats are clearly defined.

I've added til::bit_cast and replaced the reinterpret_cast hack.

DHowett · 2021-10-11T15:40:27Z

src/inc/til/bit.h

+#ifdef __cpp_lib_bit_cast
+#warning "Replace til::bit_cast and __builtin_bit_cast with std::bit_cast"
+#endif
+        return __builtin_bit_cast(To, _Val);


DHowett · 2021-10-11T15:45:42Z

@lhecker the build is still failing, fyi. 😄

This commit enables /fp:fast. This doubles the performance of the Delta E computation in #11095 for instance. Additionally it re-enables two options for debug builds which are normally enabled by default by Visual Studio. ## PR Checklist * [x] I work here * [x] Tests added/passed ## Validation Steps Performed * No change in binary size * No obvious change in behavior

Enable fast floating point model and fast debug linking

7c07c3c

lhecker requested a review from DHowett October 8, 2021 20:41

DHowett approved these changes Oct 8, 2021

View reviewed changes

carlos-zamora approved these changes Oct 8, 2021

View reviewed changes

ghost added the AutoMerge Marked for automatic merge by the bot when requirements are met label Oct 8, 2021

lhecker added 2 commits October 9, 2021 00:50

Introduce ULP based float comparisons

ccff6f6

Fix formatting

ac7c7c2

This comment has been minimized.

Sign in to view

lhecker added 2 commits October 9, 2021 00:56

Fix spellcheck

f92ccba

Fix build error

30eede7

DHowett requested changes Oct 9, 2021

View reviewed changes

ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Oct 9, 2021

DHowett reviewed Oct 9, 2021

View reviewed changes

ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Oct 9, 2021

lhecker added 2 commits October 9, 2021 03:19

Remove accidential comment

4948902

Replaced reinterpret_cast hack with til::bit_cast

75441d6

DHowett reviewed Oct 11, 2021

View reviewed changes

DHowett approved these changes Oct 11, 2021

View reviewed changes

DHowett added the zPreview-Service-Consider label Oct 11, 2021

DHowett added the zStable-Service-Consider label Oct 11, 2021

DHowett removed zPreview-Service-Consider labels Oct 11, 2021

Fix build error again

3971782

ghost merged commit b036cab into main Oct 11, 2021

ghost deleted the dev/lhecker/compiler-options branch October 11, 2021 21:02

This pull request was closed.

Conversation

lhecker commented Oct 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

Validation Steps Performed

Uh oh!

DHowett left a comment

Choose a reason for hiding this comment

Uh oh!

DHowett commented Oct 8, 2021

Uh oh!

DHowett commented Oct 8, 2021

Uh oh!

ghost commented Oct 8, 2021

Uh oh!

Rosefield commented Oct 8, 2021

Uh oh!

lhecker commented Oct 8, 2021

Uh oh!

DHowett commented Oct 8, 2021

Uh oh!

lhecker commented Oct 8, 2021

Uh oh!

This comment has been minimized.

This comment has been minimized.

lhecker commented Oct 9, 2021

Uh oh!

lhecker commented Oct 9, 2021

Uh oh!

ghost commented Oct 9, 2021

Uh oh!

DHowett left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DHowett Oct 9, 2021

Choose a reason for hiding this comment

Uh oh!

DHowett Oct 9, 2021

Choose a reason for hiding this comment

Uh oh!

lhecker Oct 9, 2021

Choose a reason for hiding this comment

Uh oh!

lhecker Oct 9, 2021

Choose a reason for hiding this comment

Uh oh!

lhecker Oct 11, 2021

Choose a reason for hiding this comment

Uh oh!

DHowett Oct 11, 2021

Choose a reason for hiding this comment

Uh oh!

DHowett commented Oct 11, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lhecker commented Oct 8, 2021 •

edited

Loading