Don't define log2 and log2f in blink

Don't define log2 and log2f in blink

The consequences are surprising.

A long time ago VC++'s CRT didn't supply log2 and log2f so WebKit/blink
supplied them as inline functions in MathExtras.h. In some cases
(including the full official builds that we ship to customers) this
meant that when Skia called log2f they ended up pulling in blink's
version. This pulled in PeriodicWave.obj which then pulled in various
other object files, and somehow the unreferenced-code logic in the
linker was unable to get rid of all of the rubbish. I noticed this
because during the gyp->gn transition I noticed that ff_cos_131072
and friends were in gn builds of chrome.dll but not gyp builds. I
fixed this but then they came back in full LTCG official builds
(full_wpo_on_official = true). Analysis of a verbose link gave this
chain of events for why rdft.obj (the definer of ff_*) was pulled in:

rdft.obj referenced by
        avfft.obj for symbol ff_rdft_init

avfft.obj referenced by
        FFTFrameFFMPEG.obj for symbol av_rdft_calc

FFTFrameFFMPEG.obj referenced by
        PeriodicWave.obj for symbol public: void __thiscall blink::FFTFrame::doInverseFFT(float *)

PeriodicWave.obj referenced by
        SkGeometry.obj for symbol log2f

Usually the trick to resolving these unwanted symbols is changing a
source_set to static_library, but this time it was recognizing that
SkGeometry has no business pulling in PeriodicWave, and removing the
VC++ definition of log2f that is the culprit. The effect on the size of
chrome.dll is:

chrome.dll
     .text:     -6656 bytes change
    .rdata:       240 bytes change
     .data:   -786624 bytes change
   .rodata:      -592 bytes change
    .reloc:      -872 bytes change
Total change: -794504 bytes

BUG=630755
Committed: https://crrev.com/036e2ceb51ebaed01c6ad552fffc227be20c655e
Cr-Commit-Position: refs/heads/master@{#437181}

[brucedawson, 2016-12-08 01:40:59]

The CQ bit was checked by brucedawson@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-08 01:41:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-08 03:26:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-08 03:26:39]

Dry run: This issue passed the CQ dry run.

[brucedawson, 2016-12-08 03:55:23]

Description was changed from

==========
Don't define log2 and log2f in Webkit

The consequences are surprising.

BUG=630755
==========

to

==========
Don't define log2 and log2f in blink

The consequences are surprising.

A long time ago VC++'s CRT didn't supply log2 and log2f so WebKit/blink
supplied them as inline functions in MathExtras.h. In some cases
(including the full official builds that we ship to customers) this
meant that when Skia called log2f they ended up pulling in blink's
version. This pulled in PeriodicWave.obj which then pulled in various
other object files, and somehow the unreferenced-code logic in the
linker was unable to get rid of all of the rubbish. I noticed this
because during the gyp->gn transition I noticed that ff_cos_131072
and friends were in gn builds of chrome.dll but not gyp builds. I
fixed this but then they came back in full LTCG official builds
(full_wpo_on_official = true). Analysis of a verbose link gave this
chain of events for why rdft.obj (the definer of ff_*) was pulled in:

rdft.obj referenced by
        avfft.obj for symbol ff_rdft_init

avfft.obj referenced by
        FFTFrameFFMPEG.obj for symbol av_rdft_calc

FFTFrameFFMPEG.obj referenced by
        PeriodicWave.obj for symbol public: void __thiscall
blink::FFTFrame::doInverseFFT(float *)

PeriodicWave.obj referenced by
        SkGeometry.obj for symbol log2f

Usually the trick to resolving these unwanted symbols is changing a
source_set to static_library, but this time it was recognizing that
SkGeometry has no business pulling in PeriodicWave, and removing the
VC++ definition of log2f that is the culprit. The effect on the size of
chrome.dll is:

chrome.dll
     .text:     -6656 bytes change
    .rdata:       240 bytes change
     .data:   -786624 bytes change
   .rodata:      -592 bytes change
    .reloc:      -872 bytes change
Total change: -794504 bytes

BUG=630755
==========

[brucedawson, 2016-12-08 03:57:31]

brucedawson@chromium.org changed reviewers:
+ thakis@chromium.org

[brucedawson, 2016-12-08 03:57:32]

PTAL

I'm going to run tests to see if these can be removed for Android as well, but
they definitely need removing for Windows.

[Nico, 2016-12-08 04:14:15]

On 2016/12/08 03:57:32, brucedawson wrote:
> PTAL
> 
> I'm going to run tests to see if these can be removed for Android as well, but
> they definitely need removing for Windows.

Oh wow, nice find and write up :-) lgtm!

[brucedawson, 2016-12-08 04:45:28]

The CQ bit was checked by brucedawson@chromium.org

[commit-bot: I haz the power, 2016-12-08 04:45:50]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-08 04:49:44]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1481172328087010, "parent_rev":
"cdf8cf1323688698bb23a92a7f0d78d2e07de8f0", "commit_rev":
"8ba3fb8867b13807ab2711286899a12a9ec8fcbb"}

[commit-bot: I haz the power, 2016-12-08 04:50:10]

Description was changed from

==========
Don't define log2 and log2f in blink

The consequences are surprising.

A long time ago VC++'s CRT didn't supply log2 and log2f so WebKit/blink
supplied them as inline functions in MathExtras.h. In some cases
(including the full official builds that we ship to customers) this
meant that when Skia called log2f they ended up pulling in blink's
version. This pulled in PeriodicWave.obj which then pulled in various
other object files, and somehow the unreferenced-code logic in the
linker was unable to get rid of all of the rubbish. I noticed this
because during the gyp->gn transition I noticed that ff_cos_131072
and friends were in gn builds of chrome.dll but not gyp builds. I
fixed this but then they came back in full LTCG official builds
(full_wpo_on_official = true). Analysis of a verbose link gave this
chain of events for why rdft.obj (the definer of ff_*) was pulled in:

rdft.obj referenced by
        avfft.obj for symbol ff_rdft_init

avfft.obj referenced by
        FFTFrameFFMPEG.obj for symbol av_rdft_calc

FFTFrameFFMPEG.obj referenced by
        PeriodicWave.obj for symbol public: void __thiscall
blink::FFTFrame::doInverseFFT(float *)

PeriodicWave.obj referenced by
        SkGeometry.obj for symbol log2f

Usually the trick to resolving these unwanted symbols is changing a
source_set to static_library, but this time it was recognizing that
SkGeometry has no business pulling in PeriodicWave, and removing the
VC++ definition of log2f that is the culprit. The effect on the size of
chrome.dll is:

chrome.dll
     .text:     -6656 bytes change
    .rdata:       240 bytes change
     .data:   -786624 bytes change
   .rodata:      -592 bytes change
    .reloc:      -872 bytes change
Total change: -794504 bytes

BUG=630755
==========

to

==========
Don't define log2 and log2f in blink

The consequences are surprising.

A long time ago VC++'s CRT didn't supply log2 and log2f so WebKit/blink
supplied them as inline functions in MathExtras.h. In some cases
(including the full official builds that we ship to customers) this
meant that when Skia called log2f they ended up pulling in blink's
version. This pulled in PeriodicWave.obj which then pulled in various
other object files, and somehow the unreferenced-code logic in the
linker was unable to get rid of all of the rubbish. I noticed this
because during the gyp->gn transition I noticed that ff_cos_131072
and friends were in gn builds of chrome.dll but not gyp builds. I
fixed this but then they came back in full LTCG official builds
(full_wpo_on_official = true). Analysis of a verbose link gave this
chain of events for why rdft.obj (the definer of ff_*) was pulled in:

rdft.obj referenced by
        avfft.obj for symbol ff_rdft_init

avfft.obj referenced by
        FFTFrameFFMPEG.obj for symbol av_rdft_calc

FFTFrameFFMPEG.obj referenced by
        PeriodicWave.obj for symbol public: void __thiscall
blink::FFTFrame::doInverseFFT(float *)

PeriodicWave.obj referenced by
        SkGeometry.obj for symbol log2f

Usually the trick to resolving these unwanted symbols is changing a
source_set to static_library, but this time it was recognizing that
SkGeometry has no business pulling in PeriodicWave, and removing the
VC++ definition of log2f that is the culprit. The effect on the size of
chrome.dll is:

chrome.dll
     .text:     -6656 bytes change
    .rdata:       240 bytes change
     .data:   -786624 bytes change
   .rodata:      -592 bytes change
    .reloc:      -872 bytes change
Total change: -794504 bytes

BUG=630755
==========

[commit-bot: I haz the power, 2016-12-08 04:50:11]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-12-08 04:52:44]

Description was changed from

==========
Don't define log2 and log2f in blink

The consequences are surprising.

A long time ago VC++'s CRT didn't supply log2 and log2f so WebKit/blink
supplied them as inline functions in MathExtras.h. In some cases
(including the full official builds that we ship to customers) this
meant that when Skia called log2f they ended up pulling in blink's
version. This pulled in PeriodicWave.obj which then pulled in various
other object files, and somehow the unreferenced-code logic in the
linker was unable to get rid of all of the rubbish. I noticed this
because during the gyp->gn transition I noticed that ff_cos_131072
and friends were in gn builds of chrome.dll but not gyp builds. I
fixed this but then they came back in full LTCG official builds
(full_wpo_on_official = true). Analysis of a verbose link gave this
chain of events for why rdft.obj (the definer of ff_*) was pulled in:

rdft.obj referenced by
        avfft.obj for symbol ff_rdft_init

avfft.obj referenced by
        FFTFrameFFMPEG.obj for symbol av_rdft_calc

FFTFrameFFMPEG.obj referenced by
        PeriodicWave.obj for symbol public: void __thiscall
blink::FFTFrame::doInverseFFT(float *)

PeriodicWave.obj referenced by
        SkGeometry.obj for symbol log2f

Usually the trick to resolving these unwanted symbols is changing a
source_set to static_library, but this time it was recognizing that
SkGeometry has no business pulling in PeriodicWave, and removing the
VC++ definition of log2f that is the culprit. The effect on the size of
chrome.dll is:

chrome.dll
     .text:     -6656 bytes change
    .rdata:       240 bytes change
     .data:   -786624 bytes change
   .rodata:      -592 bytes change
    .reloc:      -872 bytes change
Total change: -794504 bytes

BUG=630755
==========

to

==========
Don't define log2 and log2f in blink

The consequences are surprising.

A long time ago VC++'s CRT didn't supply log2 and log2f so WebKit/blink
supplied them as inline functions in MathExtras.h. In some cases
(including the full official builds that we ship to customers) this
meant that when Skia called log2f they ended up pulling in blink's
version. This pulled in PeriodicWave.obj which then pulled in various
other object files, and somehow the unreferenced-code logic in the
linker was unable to get rid of all of the rubbish. I noticed this
because during the gyp->gn transition I noticed that ff_cos_131072
and friends were in gn builds of chrome.dll but not gyp builds. I
fixed this but then they came back in full LTCG official builds
(full_wpo_on_official = true). Analysis of a verbose link gave this
chain of events for why rdft.obj (the definer of ff_*) was pulled in:

rdft.obj referenced by
        avfft.obj for symbol ff_rdft_init

avfft.obj referenced by
        FFTFrameFFMPEG.obj for symbol av_rdft_calc

FFTFrameFFMPEG.obj referenced by
        PeriodicWave.obj for symbol public: void __thiscall
blink::FFTFrame::doInverseFFT(float *)

PeriodicWave.obj referenced by
        SkGeometry.obj for symbol log2f

Usually the trick to resolving these unwanted symbols is changing a
source_set to static_library, but this time it was recognizing that
SkGeometry has no business pulling in PeriodicWave, and removing the
VC++ definition of log2f that is the culprit. The effect on the size of
chrome.dll is:

chrome.dll
     .text:     -6656 bytes change
    .rdata:       240 bytes change
     .data:   -786624 bytes change
   .rodata:      -592 bytes change
    .reloc:      -872 bytes change
Total change: -794504 bytes

BUG=630755

Committed: https://crrev.com/036e2ceb51ebaed01c6ad552fffc227be20c655e
Cr-Commit-Position: refs/heads/master@{#437181}
==========

[commit-bot: I haz the power, 2016-12-08 04:52:45]

Patchset 1 (id:??) landed as
https://crrev.com/036e2ceb51ebaed01c6ad552fffc227be20c655e
Cr-Commit-Position: refs/heads/master@{#437181}