1087873 - ~70 lines of build warning output for SIMD.cpp, starting with "SIMD.cpp:720:30: warning: comparison of integers of different signs: 'int32_t' (aka 'int') and 'const unsigned int' [-Wsign-compare]"

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Daniel Holbert [:dholbert]]

Just noticed a bunch of build warnings for SIMD.cpp. Before bug 1083238, this file (and all of js/src, I think) was warning-free. But starting with mozilla-central changeset 5e32e458318f this one file prints out ~70 lines of build warnings.

I'm building with clang version 3.5, on Ubuntu 14.10 64-bit.

Build warnings output is as follows:
{
 0:07.41 Warning: -Wsign-compare in /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/mozilla/js/src/builtin/SIMD.cpp: comparison of integers of different signs: 'int32_t' (aka 'int') and 'const unsigned int'
 0:07.41 /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/mozilla/js/src/builtin/SIMD.cpp:720:30: warning: comparison of integers of different signs: 'int32_t' (aka 'int') and 'const unsigned int' [-Wsign-compare]
 0:07.41         if (lane < 0 || lane >= V::lanes)
 0:07.41                         ~~~~ ^  ~~~~~~~~
 0:07.41 /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/mozilla/js/src/builtin/SIMD.cpp:980:1: note: in instantiation of function template specialization 'Swizzle<js::Float32x4>' requested here
 0:07.41 FLOAT32X4_FUNCTION_LIST(DEFINE_SIMD_FLOAT32X4_FUNCTION)
 0:07.41 ^
 0:07.41 /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/mozilla/js/src/builtin/SIMD.h:70:3: note: expanded from macro 'FLOAT32X4_FUNCTION_LIST'
 0:07.41   FLOAT32X4_SHUFFLE_FUNCTION_LIST(V)
 0:07.41   ^
 0:07.41 /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/mozilla/js/src/builtin/SIMD.h:62:14: note: expanded from macro 'FLOAT32X4_SHUFFLE_FUNCTION_LIST'
 0:07.41   V(swizzle, Swizzle<Float32x4>, 2, 0)                                              \
 0:07.41              ^
 0:07.41 /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/mozilla/js/src/builtin/SIMD.cpp:978:12: note: expanded from macro 'DEFINE_SIMD_FLOAT32X4_FUNCTION'
 0:07.41     return Func(cx, argc, vp);                                      \
 0:07.41            ^
 0:07.41 Warning: -Wsign-compare in /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/mozilla/js/src/builtin/SIMD.cpp: comparison of integers of different signs: 'int32_t' (aka 'int') and 'unsigned int'
 0:07.41 /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/mozilla/js/src/builtin/SIMD.cpp:749:30: warning: comparison of integers of different signs: 'int32_t' (aka 'int') and 'unsigned int' [-Wsign-compare]
 0:07.41         if (lane < 0 || lane >= (2 * V::lanes))
 0:07.41                         ~~~~ ^   ~~~~~~~~~~~~
 0:07.41 /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/mozilla/js/src/builtin/SIMD.cpp:980:1: note: in instantiation of function template specialization 'Shuffle<js::Float32x4>' requested here
 0:07.41 FLOAT32X4_FUNCTION_LIST(DEFINE_SIMD_FLOAT32X4_FUNCTION)
 0:07.41 ^
 0:07.41 /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/mozilla/js/src/builtin/SIMD.h:70:3: note: expanded from macro 'FLOAT32X4_FUNCTION_LIST'
 0:07.41   FLOAT32X4_SHUFFLE_FUNCTION_LIST(V)
 0:07.41   ^
 0:07.41 /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/mozilla/js/src/builtin/SIMD.h:63:14: note: expanded from macro 'FLOAT32X4_SHUFFLE_FUNCTION_LIST'
 0:07.41   V(shuffle, Shuffle<Float32x4>, 3, 0)
 0:07.41              ^
 0:07.41 /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/mozilla/js/src/builtin/SIMD.cpp:978:12: note: expanded from macro 'DEFINE_SIMD_FLOAT32X4_FUNCTION'
 0:07.41     return Func(cx, argc, vp);                                      \
 0:07.41            ^
 0:07.41 Warning: -Wsign-compare in /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/mozilla/js/src/builtin/SIMD.cpp: comparison of integers of different signs: 'int32_t' (aka 'int') and 'const unsigned int'
 0:07.41 /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/mozilla/js/src/builtin/SIMD.cpp:759:40: warning: comparison of integers of different signs: 'int32_t' (aka 'int') and 'const unsigned int' [-Wsign-compare]
 0:07.41         Elem *selectedInput = lanes[i] < V::lanes ? lhs : rhs;
 0:07.41                               ~~~~~~~~ ^ ~~~~~~~~
 0:07.43 Warning: -Wsign-compare in /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/mozilla/js/src/builtin/SIMD.cpp: comparison of integers of different signs: 'int32_t' (aka 'int') and 'const unsigned int'
 0:07.43 /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/mozilla/js/src/builtin/SIMD.cpp:720:30: warning: comparison of integers of different signs: 'int32_t' (aka 'int') and 'const unsigned int' [-Wsign-compare]
 0:07.43         if (lane < 0 || lane >= V::lanes)
 0:07.43                         ~~~~ ^  ~~~~~~~~
 0:07.43 /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/mozilla/js/src/builtin/SIMD.cpp:989:1: note: in instantiation of function template specialization 'Swizzle<js::Int32x4>' requested here
 0:07.43 INT32X4_FUNCTION_LIST(DEFINE_SIMD_INT32X4_FUNCTION)
 0:07.43 ^
 0:07.43 /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/mozilla/js/src/builtin/SIMD.h:120:3: note: expanded from macro 'INT32X4_FUNCTION_LIST'
 0:07.43   INT32X4_SHUFFLE_FUNCTION_LIST(V)
 0:07.43   ^
 0:07.43 /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/mozilla/js/src/builtin/SIMD.h:111:14: note: expanded from macro 'INT32X4_SHUFFLE_FUNCTION_LIST'
 0:07.43   V(swizzle, Swizzle<Int32x4>, 2, 0)                                                \
 0:07.43              ^
 0:07.43 /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/mozilla/js/src/builtin/SIMD.cpp:987:12: note: expanded from macro 'DEFINE_SIMD_INT32X4_FUNCTION'
 0:07.43     return Func(cx, argc, vp);                                      \
 0:07.43            ^
 0:07.43 Warning: -Wsign-compare in /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/mozilla/js/src/builtin/SIMD.cpp: comparison of integers of different signs: 'int32_t' (aka 'int') and 'unsigned int'
 0:07.43 /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/mozilla/js/src/builtin/SIMD.cpp:749:30: warning: comparison of integers of different signs: 'int32_t' (aka 'int') and 'unsigned int' [-Wsign-compare]
 0:07.43         if (lane < 0 || lane >= (2 * V::lanes))
 0:07.43                         ~~~~ ^   ~~~~~~~~~~~~
 0:07.43 /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/mozilla/js/src/builtin/SIMD.cpp:989:1: note: in instantiation of function template specialization 'Shuffle<js::Int32x4>' requested here
 0:07.43 INT32X4_FUNCTION_LIST(DEFINE_SIMD_INT32X4_FUNCTION)
 0:07.43 ^
 0:07.43 /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/mozilla/js/src/builtin/SIMD.h:120:3: note: expanded from macro 'INT32X4_FUNCTION_LIST'
 0:07.43   INT32X4_SHUFFLE_FUNCTION_LIST(V)
 0:07.43   ^
 0:07.43 /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/mozilla/js/src/builtin/SIMD.h:112:14: note: expanded from macro 'INT32X4_SHUFFLE_FUNCTION_LIST'
 0:07.43   V(shuffle, Shuffle<Int32x4>, 3, 0)
 0:07.43              ^
 0:07.43 /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/mozilla/js/src/builtin/SIMD.cpp:987:12: note: expanded from macro 'DEFINE_SIMD_INT32X4_FUNCTION'
 0:07.43     return Func(cx, argc, vp);                                      \
 0:07.43            ^
 0:07.43 Warning: -Wsign-compare in /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/mozilla/js/src/builtin/SIMD.cpp: comparison of integers of different signs: 'int32_t' (aka 'int') and 'const unsigned int'
 0:07.43 /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/mozilla/js/src/builtin/SIMD.cpp:759:40: warning: comparison of integers of different signs: 'int32_t' (aka 'int') and 'const unsigned int' [-Wsign-compare]
 0:07.43         Elem *selectedInput = lanes[i] < V::lanes ? lhs : rhs;
 0:07.43                               ~~~~~~~~ ^ ~~~~~~~~
}

Comment 1

[Daniel Holbert [:dholbert]]

I think there are only 6 actual warnings in there -- they are:
{
SIMD.cpp:720:30: warning: comparison of integers of different signs: 'int32_t' (aka 'int') and 'const unsigned int' [-Wsign-compare]
        if (lane < 0 || lane >= V::lanes)
                        ~~~~ ^  ~~~~~~~~

SIMD.cpp:749:30: warning: comparison of integers of different signs: 'int32_t' (aka 'int') and 'unsigned int' [-Wsign-compare]
        if (lane < 0 || lane >= (2 * V::lanes))
                        ~~~~ ^   ~~~~~~~~~~~~


SIMD.cpp:759:40: warning: comparison of integers of different signs: 'int32_t' (aka 'int') and 'const unsigned int' [-Wsign-compare]
        Elem *selectedInput = lanes[i] < V::lanes ? lhs : rhs;
                              ~~~~~~~~ ^ ~~~~~~~~


SIMD.cpp:720:30: warning: comparison of integers of different signs: 'int32_t' (aka 'int') and 'const unsigned int' [-Wsign-compare]
        if (lane < 0 || lane >= V::lanes)
                        ~~~~ ^  ~~~~~~~~


SIMD.cpp:749:30: warning: comparison of integers of different signs: 'int32_t' (aka 'int') and 'unsigned int' [-Wsign-compare]
        if (lane < 0 || lane >= (2 * V::lanes))
                        ~~~~ ^   ~~~~~~~~~~~~

SIMD.cpp:759:40: warning: comparison of integers of different signs: 'int32_t' (aka 'int') and 'const unsigned int' [-Wsign-compare]
        Elem *selectedInput = lanes[i] < V::lanes ? lhs : rhs;
                              ~~~~~~~~ ^ ~~~~~~~~
}

(The rest of the output is backtracing through template expansion, I think.)

Comment 2

[Daniel Holbert [:dholbert]]

Based on other lines in this file (e.g. a loop with "unsigned i = 0; i < V::lanes;"), it looks like V::lanes is known to be unsigned.  So comparing a signed variable against it is somewhat bogus.

Maybe "lane" should have type "unsigned" or "uint32_t", and have its sentinel value be the corresponding MAX value for its type?

Comment 3

[Daniel Holbert [:dholbert]]

(It looks like it might only have type int32_t right now so that we can initialize it with a sentinel value of -1.)

Comment 4

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: Fix signed/unsigned comparisons in Shuffle/Swizzle

Thanks for the heads up! gcc didn't complain at all. I really need to use clang
by default.

Comment 5

[Till Schneidereit [:till]]

Comment on attachment 8510962 [details] [diff] [review]
Fix signed/unsigned comparisons in Shuffle/Swizzle

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

Urgh, yes. I should've caught this, sorry.

Comment 6

[Benjamin Bouvier [:bbouvier] (inactive)]

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

Comment 7

[Carsten Book [:Tomcat]]

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