886481 - Do not display total compilation time when asm.js is successfully compiled, in a --enable-more-deterministic shell

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

+++ This bug was initially created as a clone of Bug #885103 +++

warning: successfully compiled asm.js code (total compilation time 0ms)

For now, bug 885103 turns all the compilation time messages to 0ms no matter what flags they are running, but Jesse prefers the message to not be there at all. See bug 885103 for the entire backstory.

Comment 1

[Sahil Chelaramani]

Attachment: Removed the string that displays the compile time for asm.js

Could you please review this patch?

Comment 2

[Luke Wagner [:luke]]

Thanks for looking at this bug.  What I think Gary wants is to remove the message only when JS_MORE_DETERMINISTIC is defined (via the --enable-more-deterministic configure flag); in normal builds we definitely want to see this information.

Comment 3

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Yep, a #ifdef/#ifndef JS_MORE_DETERMINISTIC is needed.

Comment 4

[Sahil Chelaramani]

Attachment: work.patch

I think this should do the trick, as I just shifted the print line appropriately, and it will only display the compilation time when JS_MORE_DETERMINISTIC is not defined.

Comment 5

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Comment on attachment 787102 [details] [diff] [review]
work.patch

I'm not sure if out->reset still needs to be called, so pushing to Luke for advice.

Comment 6

[Luke Wagner [:luke]]

It looks like Warn gets passed a const char * (which would be NULL) and passes that to JS_ReportErrorFlagsAndNumber (which it looks like assumes non-NULL).  So just change Warn() to pass (str ? str : "") instead.

Comment 7

[Sahil Chelaramani]

Attachment: Changed parameter being passed to JS_ReportErrorFlagsAndNumber in Warn function

This should do it.

Comment 8

[Sahil Chelaramani]

Attachment: A merged patch correcting both issues

Here is a single patch for both of the previous patches!!

Comment 9

[Sahil Chelaramani]

Attachment: Final Patch for bugfix

Final Patch for the bugfix

Comment 10

[Luke Wagner [:luke]]

Comment on attachment 787152 [details] [diff] [review]
Final Patch for bugfix

Great!

Comment 11

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Thanks Sahil!

https://hg.mozilla.org/integration/mozilla-inbound/rev/c89f13b656d8

Comment 12

[Carsten Book [:Tomcat]]

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