This is an archive of the discontinued LLVM Phabricator instance.

Differential D77393

[X86] Fix implicit sign conversion warnings in X86 headers.
ClosedPublic

Authored by pgousseau on Apr 3 2020, 6:51 AM.

Details

Reviewers
craig.topper
probinson
filcab
RKSimon
Commits
rG937e63b8d5e9: [X86] Fix x86-header-warnings.c test not detecting regressions as intended.
rG08fab9ebecf7: [X86] Fix implicit sign conversion warnings in X86 headers.
Summary

Warnings in emmintrin.h and xmmintrin.h are reported by -fsanitize=implicit-integer-sign-change

Diff Detail

Event Timeline

pgousseau created this revision.Apr 3 2020, 6:51 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 3 2020, 6:51 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
RKSimon added a reviewer: RKSimon.Apr 3 2020, 7:28 AM
RKSimon added a subscriber: RKSimon.

Rename test file to x86-header-warnings.c or something similarly specific?

clang/test/Headers/warn-cleanup.c
10 ↗(On Diff #254784)

Use x8intrin.h directly so its easier to expand in the future?

Harbormaster failed remote builds in B51629: Diff 254784!Apr 3 2020, 8:02 AM
pgousseau updated this revision to Diff 254827.Apr 3 2020, 8:46 AM

Renaming test and including x86intrin.h instead.

RKSimon added a comment.Apr 6 2020, 8:13 AM

@craig.topper Any comments? Hopefully we can add extra tests as time goes on (for instance I'm intending to add Wdocumentation test coverage for PR35949).

clang/test/Headers/x86-header-warnings.c
13

Add a LABEL check as well

craig.topper added a comment.Apr 6 2020, 10:49 AM

Seems fine to me.

RKSimon accepted this revision.Apr 6 2020, 11:41 AM

LGTM with the minor addition an extra CHECK-LABEL

This revision is now accepted and ready to land.Apr 6 2020, 11:41 AM
pgousseau updated this revision to Diff 255626.Apr 7 2020, 3:21 AM

Added label

thakis added a subscriber: thakis.Apr 7 2020, 3:38 AM
thakis added inline comments.
clang/test/Headers/x86-header-warnings.c
3

Does Wsystem-headers add too much noise?

Closed by commit rG08fab9ebecf7: [X86] Fix implicit sign conversion warnings in X86 headers. (authored by pgousseau). · Explain WhyApr 7 2020, 3:45 AM
This revision was automatically updated to reflect the committed changes.
pgousseau marked 2 inline comments as done.Apr 7 2020, 4:07 AM
pgousseau added inline comments.
clang/test/Headers/x86-header-warnings.c
3

Thanks for the suggestion, I did not thought about adding Wsystem-headers, I tested Wsystem-headers now though and cant seem to pick up on that particular issue without the preprocessing step.

13

Adding label sounds good, went ahead and commited without checking the test was not failing anymore, sorry will revert.

pgousseau reopened this revision.Apr 7 2020, 6:43 AM

Reopening to fix false negative test.

This revision is now accepted and ready to land.Apr 7 2020, 6:43 AM
pgousseau updated this revision to Diff 255666.Apr 7 2020, 6:45 AM

Fix false negative test introduce while attempting to add label check.

probinson added a comment.Apr 8 2020, 6:55 AM

It looks like you're not actually interested in the compiled output, but just whether warnings occurred; in that case you'd be better off with -verify -fsyntax-only and // expected-no-diagnostics.

pgousseau added a comment.Apr 8 2020, 7:29 AM
In D77393#1969419, @probinson wrote:

It looks like you're not actually interested in the compiled output, but just whether warnings occurred; in that case you'd be better off with -verify -fsyntax-only and // expected-no-diagnostics.

Sounds good! Committed at https://github.com/llvm/llvm-project/commit/937e63b8d5e961c2a7da25558bbcdd5388182b67
Thanks!

Closed by commit rG937e63b8d5e9: [X86] Fix x86-header-warnings.c test not detecting regressions as intended. (authored by pgousseau). · Explain WhyApr 8 2020, 7:34 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
clang/
lib/
Headers/
6 lines
50 lines
test/
Headers/
43 lines

Diff 255626

clang/lib/Headers/emmintrin.h

Show First 20 Lines • Show All 4,964 Lines • ▼ Show 20 Lines
#if defined(__cplusplus) #if defined(__cplusplus)
} // extern "C" } // extern "C"
#endif #endif
#undef __DEFAULT_FN_ATTRS #undef __DEFAULT_FN_ATTRS
#undef __DEFAULT_FN_ATTRS_MMX #undef __DEFAULT_FN_ATTRS_MMX
#define _MM_SHUFFLE2(x, y) (((x) << 1) | (y)) #define _MM_SHUFFLE2(x, y) (((x) << 1) | (y))
#define _MM_DENORMALS_ZERO_ON (0x0040) #define _MM_DENORMALS_ZERO_ON (0x0040U)
#define _MM_DENORMALS_ZERO_OFF (0x0000) #define _MM_DENORMALS_ZERO_OFF (0x0000U)
#define _MM_DENORMALS_ZERO_MASK (0x0040) #define _MM_DENORMALS_ZERO_MASK (0x0040U)
#define _MM_GET_DENORMALS_ZERO_MODE() (_mm_getcsr() & _MM_DENORMALS_ZERO_MASK) #define _MM_GET_DENORMALS_ZERO_MODE() (_mm_getcsr() & _MM_DENORMALS_ZERO_MASK)
#define _MM_SET_DENORMALS_ZERO_MODE(x) (_mm_setcsr((_mm_getcsr() & ~_MM_DENORMALS_ZERO_MASK) | (x))) #define _MM_SET_DENORMALS_ZERO_MODE(x) (_mm_setcsr((_mm_getcsr() & ~_MM_DENORMALS_ZERO_MASK) | (x)))
#endif /* __EMMINTRIN_H */ #endif /* __EMMINTRIN_H */

clang/lib/Headers/xmmintrin.h

Show First 20 Lines • Show All 2,925 Lines • ▼ Show 20 Lines_mm_movemask_ps(__m128 __a)
return __builtin_ia32_movmskps((__v4sf)__a); return __builtin_ia32_movmskps((__v4sf)__a);
} }
#define _MM_ALIGN16 __attribute__((aligned(16))) #define _MM_ALIGN16 __attribute__((aligned(16)))
#define _MM_SHUFFLE(z, y, x, w) (((z) << 6) | ((y) << 4) | ((x) << 2) | (w)) #define _MM_SHUFFLE(z, y, x, w) (((z) << 6) | ((y) << 4) | ((x) << 2) | (w))
#define _MM_EXCEPT_INVALID (0x0001) #define _MM_EXCEPT_INVALID (0x0001U)
#define _MM_EXCEPT_DENORM (0x0002) #define _MM_EXCEPT_DENORM (0x0002U)
#define _MM_EXCEPT_DIV_ZERO (0x0004) #define _MM_EXCEPT_DIV_ZERO (0x0004U)
#define _MM_EXCEPT_OVERFLOW (0x0008) #define _MM_EXCEPT_OVERFLOW (0x0008U)
#define _MM_EXCEPT_UNDERFLOW (0x0010) #define _MM_EXCEPT_UNDERFLOW (0x0010U)
#define _MM_EXCEPT_INEXACT (0x0020) #define _MM_EXCEPT_INEXACT (0x0020U)
#define _MM_EXCEPT_MASK (0x003f) #define _MM_EXCEPT_MASK (0x003fU)
#define _MM_MASK_INVALID (0x0080) #define _MM_MASK_INVALID (0x0080U)
#define _MM_MASK_DENORM (0x0100) #define _MM_MASK_DENORM (0x0100U)
#define _MM_MASK_DIV_ZERO (0x0200) #define _MM_MASK_DIV_ZERO (0x0200U)
#define _MM_MASK_OVERFLOW (0x0400) #define _MM_MASK_OVERFLOW (0x0400U)
#define _MM_MASK_UNDERFLOW (0x0800) #define _MM_MASK_UNDERFLOW (0x0800U)
#define _MM_MASK_INEXACT (0x1000) #define _MM_MASK_INEXACT (0x1000U)
#define _MM_MASK_MASK (0x1f80) #define _MM_MASK_MASK (0x1f80U)
#define _MM_ROUND_NEAREST (0x0000) #define _MM_ROUND_NEAREST (0x0000U)
#define _MM_ROUND_DOWN (0x2000) #define _MM_ROUND_DOWN (0x2000U)
#define _MM_ROUND_UP (0x4000) #define _MM_ROUND_UP (0x4000U)
#define _MM_ROUND_TOWARD_ZERO (0x6000) #define _MM_ROUND_TOWARD_ZERO (0x6000U)
#define _MM_ROUND_MASK (0x6000) #define _MM_ROUND_MASK (0x6000U)
#define _MM_FLUSH_ZERO_MASK (0x8000) #define _MM_FLUSH_ZERO_MASK (0x8000U)
#define _MM_FLUSH_ZERO_ON (0x8000) #define _MM_FLUSH_ZERO_ON (0x8000U)
#define _MM_FLUSH_ZERO_OFF (0x0000) #define _MM_FLUSH_ZERO_OFF (0x0000U)
#define _MM_GET_EXCEPTION_MASK() (_mm_getcsr() & _MM_MASK_MASK) #define _MM_GET_EXCEPTION_MASK() (_mm_getcsr() & _MM_MASK_MASK)
#define _MM_GET_EXCEPTION_STATE() (_mm_getcsr() & _MM_EXCEPT_MASK) #define _MM_GET_EXCEPTION_STATE() (_mm_getcsr() & _MM_EXCEPT_MASK)
#define _MM_GET_FLUSH_ZERO_MODE() (_mm_getcsr() & _MM_FLUSH_ZERO_MASK) #define _MM_GET_FLUSH_ZERO_MODE() (_mm_getcsr() & _MM_FLUSH_ZERO_MASK)
#define _MM_GET_ROUNDING_MODE() (_mm_getcsr() & _MM_ROUND_MASK) #define _MM_GET_ROUNDING_MODE() (_mm_getcsr() & _MM_ROUND_MASK)
#define _MM_SET_EXCEPTION_MASK(x) (_mm_setcsr((_mm_getcsr() & ~_MM_MASK_MASK) | (x))) #define _MM_SET_EXCEPTION_MASK(x) (_mm_setcsr((_mm_getcsr() & ~_MM_MASK_MASK) | (x)))
#define _MM_SET_EXCEPTION_STATE(x) (_mm_setcsr((_mm_getcsr() & ~_MM_EXCEPT_MASK) | (x))) #define _MM_SET_EXCEPTION_STATE(x) (_mm_setcsr((_mm_getcsr() & ~_MM_EXCEPT_MASK) | (x)))
▲ Show 20 Lines • Show All 42 Lines • Show Last 20 Lines

clang/test/Headers/x86-header-warnings.c

  • This file was added.
// Fix sign conversion warnings found by fsanitize=implicit-integer-sign-change
// in intrinsic headers.
// Preprocess file to workaround no warnings in system headers.
thakisUnsubmitted
Not Done
ReplyInline Actions

Does Wsystem-headers add too much noise?

thakis: Does Wsystem-headers add too much noise?
pgousseauAuthorUnsubmitted
Done
ReplyInline Actions

Thanks for the suggestion, I did not thought about adding Wsystem-headers, I tested Wsystem-headers now though and cant seem to pick up on that particular issue without the preprocessing step.

pgousseau: Thanks for the suggestion, I did not thought about adding Wsystem-headers, I tested Wsystem…
// RUN: %clang_cc1 %s -triple x86_64-pc-linux-gnu -ffreestanding -E 2>&1 \
// RUN: | %clang_cc1 -x c - -triple x86_64-pc-linux-gnu -ffreestanding -Wsign-conversion -E -o - 2>&1 \
// RUN: | FileCheck --allow-empty %s
// REQUIRES: x86-registered-target
#include <x86intrin.h>
void test0() {
// CHECK-LABEL: test0
// CHECK-NOT: warning:
RKSimonUnsubmitted
Not Done
ReplyInline Actions

Add a LABEL check as well

RKSimon: Add a LABEL check as well
pgousseauAuthorUnsubmitted
Done
ReplyInline Actions

Adding label sounds good, went ahead and commited without checking the test was not failing anymore, sorry will revert.

pgousseau: Adding label sounds good, went ahead and commited without checking the test was not failing…
_MM_SET_DENORMALS_ZERO_MODE(_MM_DENORMALS_ZERO_ON);
_MM_SET_DENORMALS_ZERO_MODE(_MM_DENORMALS_ZERO_OFF);
_MM_SET_DENORMALS_ZERO_MODE(_MM_DENORMALS_ZERO_MASK);
_MM_SET_EXCEPTION_STATE(_MM_EXCEPT_INVALID);
_MM_SET_EXCEPTION_STATE(_MM_EXCEPT_DENORM);
_MM_SET_EXCEPTION_STATE(_MM_EXCEPT_DIV_ZERO);
_MM_SET_EXCEPTION_STATE(_MM_EXCEPT_OVERFLOW);
_MM_SET_EXCEPTION_STATE(_MM_EXCEPT_UNDERFLOW);
_MM_SET_EXCEPTION_STATE(_MM_EXCEPT_INEXACT);
_MM_SET_EXCEPTION_STATE(_MM_EXCEPT_MASK);
_MM_SET_EXCEPTION_MASK(_MM_MASK_INVALID);
_MM_SET_EXCEPTION_MASK(_MM_MASK_DENORM);
_MM_SET_EXCEPTION_MASK(_MM_MASK_DIV_ZERO);
_MM_SET_EXCEPTION_MASK(_MM_MASK_OVERFLOW);
_MM_SET_EXCEPTION_MASK(_MM_MASK_UNDERFLOW);
_MM_SET_EXCEPTION_MASK(_MM_MASK_INEXACT);
_MM_SET_EXCEPTION_MASK(_MM_MASK_MASK);
_MM_SET_ROUNDING_MODE(_MM_ROUND_NEAREST);
_MM_SET_ROUNDING_MODE(_MM_ROUND_DOWN);
_MM_SET_ROUNDING_MODE(_MM_ROUND_UP);
_MM_SET_ROUNDING_MODE(_MM_ROUND_TOWARD_ZERO);
_MM_SET_ROUNDING_MODE(_MM_ROUND_MASK);
_MM_SET_FLUSH_ZERO_MODE(_MM_FLUSH_ZERO_MASK);
_MM_SET_FLUSH_ZERO_MODE(_MM_FLUSH_ZERO_ON);
_MM_SET_FLUSH_ZERO_MODE(_MM_FLUSH_ZERO_OFF);
}