Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(68)

Issue 1778653002: Add explicit includes for a few intrinsics. (Closed)

Created:
4 years, 9 months ago by Nico
Modified:
4 years, 9 months ago
Reviewers:
Roland McGrath
CC:
native-client-reviews_googlegroups.com
Base URL:
https://chromium.googlesource.com/native_client/src/native_client.git@master
Target Ref:
refs/heads/master
Project:
nacl
Visibility:
Public.

Description

Add explicit includes for a few intrinsics. __halt, __cpuid, _ReadWriteBarrier, and _mm_getcsr are all declared in intrin.h, see https://msdn.microsoft.com/en-us/library/hh977022.aspx The code currently builds fine because undeclared functions are ok in C code before C99. BUG=https://crbug.com/592745 Committed: https://chromium.googlesource.com/native_client/src/native_client/+/e7ca61ace9455831384f7d941b1d53d8ebd8770a

Patch Set 1 #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+8 lines, -1 line) Patch
M src/shared/platform/win/nacl_exit.c View 1 chunk +1 line, -0 lines 0 comments Download
M src/shared/platform/win/nacl_time.c View 1 chunk +1 line, -0 lines 3 comments Download
M src/trusted/service_runtime/arch/x86_32/sel_rt_32.c View 1 chunk +1 line, -1 line 0 comments Download
M src/trusted/service_runtime/sel_ldr.c View 1 chunk +5 lines, -0 lines 0 comments Download

Messages

Total messages: 11 (4 generated)
Nico
4 years, 9 months ago (2016-03-08 20:46:12 UTC) #3
Roland McGrath
LGTM modulo style nit. Is there a warning option we could enable that would have ...
4 years, 9 months ago (2016-03-08 21:58:25 UTC) #4
Nico
I think C4013 finds this (enabled at /W3 and up) -- but it looks like ...
4 years, 9 months ago (2016-03-08 22:03:37 UTC) #5
Roland McGrath
We can indeed look into adding a clang-cl bot. https://codereview.chromium.org/1778653002/diff/1/src/shared/platform/win/nacl_time.c File src/shared/platform/win/nacl_time.c (right): https://codereview.chromium.org/1778653002/diff/1/src/shared/platform/win/nacl_time.c#newcode16 src/shared/platform/win/nacl_time.c:16: ...
4 years, 9 months ago (2016-03-08 22:05:31 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1778653002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1778653002/1
4 years, 9 months ago (2016-03-08 22:16:34 UTC) #8
Nico
Thanks!
4 years, 9 months ago (2016-03-08 22:16:52 UTC) #9
commit-bot: I haz the power
4 years, 9 months ago (2016-03-08 22:18:14 UTC) #11
Message was sent while issue was closed.
Committed patchset #1 (id:1) as
https://chromium.googlesource.com/native_client/src/native_client/+/e7ca61ace...

Powered by Google App Engine
This is Rietveld 408576698