Closed Bug 901529 Opened 6 years ago Closed 6 years ago

content/base/src/nsTextFragmentSSE2.cpp(45) : warning C4309: 'argument' : truncation of constant value

Categories

(Core :: DOM: Core & HTML, defect)

All
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: dholbert, Assigned: dholbert)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Windows build warning:
{
content/base/src/nsTextFragmentSSE2.cpp(45) : warning C4309: 'argument' : truncation of constant value
}

Here's a recent try build (with -Werror turned on in this directory), which shows this warning:
 https://tbpl.mozilla.org/php/getParsedLog.php?id=26013956&tree=Try

It points to this line of code:
> 45   __m128i vectmask = _mm_set1_epi16(0xff00);

...and the issue is that _mm_set1_epi16 takes a *signed* short[1], whereas 0xff00 is a large unsigned short (larger than the largest signed short).

One way we could fix this is by declaring _mm_set1_epi16 as an unsigned short, and then static_casting it to short for the _mm_set1_epi16 call.

[1] http://msdn.microsoft.com/en-us/library/k0ya3x0e%28v=vs.80%29.aspx
Attached patch fix v1Splinter Review
(In reply to Daniel Holbert [:dholbert] (lowered responsiveness through 8/6) from comment #0)
> One way we could fix this is by declaring _mm_set1_epi16 as an unsigned
> short, and then static_casting it to short for the _mm_set1_epi16 call.

Here's that option, basically. Hopefully this works?  Giving it a try run now; I'll request review later if it works.
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Comment on attachment 785766 [details] [diff] [review]
fix v1

Yup, this seems to fix it.

Log of a build with the fix:
 https://tbpl.mozilla.org/php/getParsedLog.php?id=26167150&tree=Try#error0
(It's still busted because of a FAIL_ON_WARNINGS in the patch-stack plus another build warning (bug 900260), but no warnings reported for nsTextFragmentSSE2.cpp itself.)
Attachment #785766 - Attachment description: fix? → fix v1
Attachment #785766 - Flags: review?(justin.lebar+bug)
Comment on attachment 785766 [details] [diff] [review]
fix v1

Wow, I hope that wasn't affecting correctness!
Attachment #785766 - Flags: review?(justin.lebar+bug) → review+
(Pretty sure this didn't affect correctness - I think the old code ended up doing what we expect. Evidence for that: we hit a bunch of these in bug 552805, in a C++ test, a while back -- the test passed (and behavior didn't change when we fixed the warnings), even though it was hitting a ton of these warnings for all the test data.)
(er, technically the MSVC C4309 versions of those C++-test-warnings from comment 5 were reported in bug 449291, and then I filed & fixed bug 552805 for the equivalent GCC warnings, and that took care of the MSVC warnings as well. Anyway: comment 5's point stands, but just see bug 449291 for where the MSVC warnings were reported, for anyone who cares.)
https://hg.mozilla.org/mozilla-central/rev/f803e142efc4
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.