Summary: | Fix build: using integer absolute value function 'abs' when argument is of floating point type | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | David Kilzer (:ddkilzer) <ddkilzer> | ||||
Component: | JavaScriptCore | Assignee: | David Kilzer (:ddkilzer) <ddkilzer> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | fpizlo, mhahnenberg, msaboff | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
David Kilzer (:ddkilzer)
2014-03-14 22:04:28 PDT
Created attachment 226808 [details]
Patch v1
Comment on attachment 226808 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=226808&action=review R=me to fix build. Leaving cq? alone in case you want to land the fabs() change instead. Calling abs() and casting to int means we miss a *bunch* of blinding opportunities. > Source/JavaScriptCore/assembler/MacroAssembler.h:992 > - value = abs(value); > + value = abs(static_cast<int>(value)); I strongly suspect that we meant to call fabs instead. (In reply to comment #2) > (From update of attachment 226808 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=226808&action=review > > R=me to fix build. Leaving cq? alone in case you want to land the fabs() change instead. This is for a trunk version of clang we're not compiling with yet, so no need to rush a fix. > Calling abs() and casting to int means we miss a *bunch* of blinding opportunities. > > > Source/JavaScriptCore/assembler/MacroAssembler.h:992 > > - value = abs(value); > > + value = abs(static_cast<int>(value)); > > I strongly suspect that we meant to call fabs instead. That's why I posted a patch for review. :) I'll switch to using fabs() before I land the patch. Thanks! Committed r165683: <http://trac.webkit.org/changeset/165683> Comment on attachment 226808 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=226808&action=review > Source/JavaScriptCore/ChangeLog:10 > + JavaScriptCore/assembler/MacroAssembler.h:992:17: error: using integer absolute value function 'abs' when argument is of floating point type [-Werror,-Wabsolute-value] I am *so* glad they added this warning. Use of abs instead of fabs is something I keep catching in patch review. |