1303463 - Use safer STRICT_ASSIGN on VisualStudio.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Tooru Fujisawa [:arai]]

As discovered in bug 1303085, VisualStudio doesn't do STRICT_ASSIGN as expected on float on some case.
changing it to use safer version solves the issue.

Comment 1

[Tooru Fujisawa [:arai]]

testing a patch with bug 1303085.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2950d3c9fdb6

Comment 2

[Tooru Fujisawa [:arai]]

Attachment: Use safer STRICT_ASSIGN on VisualStudio.

Can I have your feedback on this approach?
I'll check performance impact shortly.

Comment 3

[Tooru Fujisawa [:arai]]

triggered PGO builds for before/after

  https://treeherder.mozilla.org/#/jobs?repo=try&revision=17ca54bea9fa9becb67f258b6d483b3ce79da211
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=a2f154f135d37125b788ee3fb2b96224a59624c7

Comment 4

[Tooru Fujisawa [:arai]]

I don't see any performance difference between them, with the following code:

function f() {
  var y = 0;
  for (var x = -100; x <= 100; x += 0.0001) {
    y += Math.exp(x) * 0.00000001;
  }
  return y;
}

result on my windows 10 laptop (32bit)
  before: 617.4ms
  after:  614.1ms

we could go with it.

Comment 5

[Tooru Fujisawa [:arai]]

Comment on attachment 8792166 [details] [diff] [review]
Use safer STRICT_ASSIGN on VisualStudio.

so, changing to safer STRICT_ASSIGN doesn't introduce any performance issue,
and it solves the issue in bug 1303085.

Comment 6

[Tooru Fujisawa [:arai]]

so far, I confirmed the issue about correctness only with float type, and we don't have STRICT_ASSIGN with float in tree.
so this issue won't affect existing functions.

Comment 7

[Jeff Walden [:Waldo]]

Comment on attachment 8792166 [details] [diff] [review]
Use safer STRICT_ASSIGN on VisualStudio.

Review of attachment 8792166 [details] [diff] [review]:
-----------------------------------------------------------------

This is, um, super rubberstampy.

Comment 8

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e407c6185ace3581212d8ba05cc7ec27ca563d11
Bug 1303463 - Use safer STRICT_ASSIGN on VisualStudio. r=jwalden

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e407c6185ace

Comment 10

[Gerry Chang [:gchang]]

Mark 51 as won't fix. If it's worth uplifting to 51, feel free to nominate it.