Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2076483002:
clang/win: Stop force-including intrin.h everywhere. (Closed)
Descriptionclang/win: Stop force-including intrin.h everywhere.
MSVC and clang-cl offer a bunch of functions that are built-in to the compiler: Intrinsics.
When an intrinsic is called, the compiler can emit specialized code for intrinsics.
(It's free to emit a regular call to a function too though.)
intrin.h includes prototypes (and with clang-cl, definitions) for intrinsics. MSVC also supports
`#pragma intrinsics(name)`, which allows marking the intrinsic named "name" as something
that should preferably be treated as an intrinsic (roughly ~inlined) instead of as a call. With this,
it is possible to manually declare an intrinsic and then use that pragma, and since it'll be
treated as an intrinsic, no linker error will happen.
clang-cl doesn't yet implement `#pragma intrinsic`, so e.g.
void __cpuidex(int CPUInfo[4], int info_type, int ecxvalue);
#pragma intrinsic(__cpuidex)
// later, call __cpuidex()
will result in a linker error, since clang-cl sees the declaration for __cpuidex(), but then
clang-cl ignores the pragma line, and then later it thinks the call is just a call to a function
that isn't defined anywhere.
This is not a problem: Just #include <intrin.h> instead of manually declaring the function.
With clang-cl, intrin.h contains a definition of __cpuidex (and the other intrinsics), and
no linker error will be emitted (and the definition is always_inline, so it's fast).
There is just one wrinkle: Some system headers (e.g. windows.h) do manually declare
intrinsics, mark them `#pragma intrinsic`, and then call them from other inline functions
defined in system headers. So if some of our code calls one of these other inline functions,
it needs intrin.h without us knowing about it. Luckily, we know only a single function where
this is an issue in practice: SecureZeroMemory(), which calls __stosb. So we had to add
explicit and mysterious includes for <intrin.h> to the two files that call that function. If
more examples of this pop up, we can reevaluate if we want to force-include this header
everywhere, but for now it seems overkill to inject this header into every translation unit
just because two translation units need it.
BUG=592745
Committed: https://crrev.com/ac7670452f51ecfd1061749b3d5e15f8544a55ee
Cr-Commit-Position: refs/heads/master@{#401613}
Patch Set 1 #
Messages
Total messages: 21 (11 generated)
Description was changed from ========== clang/win: Stop force-including intrin.h everywhere. XXX explain what this means BUG= ========== to ========== clang/win: Stop force-including intrin.h everywhere. XXX explain what this means BUG=592745 ==========
The CQ bit was checked by thakis@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2076483002/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_clang on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)
Description was changed from ========== clang/win: Stop force-including intrin.h everywhere. XXX explain what this means BUG=592745 ========== to ========== clang/win: Stop force-including intrin.h everywhere. MSVC and clang-cl offer a bunch of functions that are built-in to the compiler: Intrinsics. When an intrinsic is called, the compiler can emit specialized code for intrinsics. (It's free to emit a regular call to a function too though.) intrin.h includes prototypes (and with clang-cl, definitions) for intrinsics. MSVC also supports `#pragma intrinsics(name)`, which allows marking the intrinsic named "name" as something that should preferably be treated as an intrinsic (roughly ~inlined) instead of as a call. With this, it is possible to manually declare an intrinsic and then use that pragma, and since it'll be treated as an intrinsic, no linker error will happen. clang-cl doesn't yet implement `#pragma intrinsic`, so e.g. void __cpuidex(int CPUInfo[4], int info_type, int ecxvalue); #pragma intrinsic(__cpuidex) // later, call __cpuidex() will result in a linker error, since clang-cl sees the declaration for __cpuidex(), but then clang-cl ignores the pragma line, and then later it thinks the call is just a call to a function that isn't defined anywhere. This is not a problem: Just #include <intrin.h> instead of manually declaring the function. With clang-cl, intrin.h contains a definition of __cpuidex (and the other intrinsics), and no linker error will be emitted (and the definition is always_inline, so it's fast). There is just one wrinkle: Some system headers (e.g. windows.h) do manually declare intrinsics, mark them `#pragma intrinsic`, and then call them from other inline functions defined in system headers. So if some of our code calls one of these other inline functions, it needs intrin.h without us knowing about it. Luckily, we know only a single function where this is an issue in practice: SecureZeroMemory(), which calls __stosb. So we had to add explicit and mysterious includes for <intrin.h> to the two files that call that function. If more examples of this pop up, we can reevaluate if we want to stop force-including this header everywhere, but for now it seems overkill to inject this header into every translation unit just because two translation units need it. BUG=592745 ==========
thakis@chromium.org changed reviewers: + hans@chromium.org
The CQ bit was checked by thakis@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2076483002/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Nice! LGTM, and thanks for the excellent change description! Nit: > If more examples of this pop up, we can reevaluate if we want to stop force-including this header everywhere, but for now it seems overkill to inject this header into every translation unit just because two translation units need it. I'd leave out "stop" from "reevaluate if we want to stop force-including", because the reevaluating would really be about whether to force-include or not, no need to negate.
Description was changed from ========== clang/win: Stop force-including intrin.h everywhere. MSVC and clang-cl offer a bunch of functions that are built-in to the compiler: Intrinsics. When an intrinsic is called, the compiler can emit specialized code for intrinsics. (It's free to emit a regular call to a function too though.) intrin.h includes prototypes (and with clang-cl, definitions) for intrinsics. MSVC also supports `#pragma intrinsics(name)`, which allows marking the intrinsic named "name" as something that should preferably be treated as an intrinsic (roughly ~inlined) instead of as a call. With this, it is possible to manually declare an intrinsic and then use that pragma, and since it'll be treated as an intrinsic, no linker error will happen. clang-cl doesn't yet implement `#pragma intrinsic`, so e.g. void __cpuidex(int CPUInfo[4], int info_type, int ecxvalue); #pragma intrinsic(__cpuidex) // later, call __cpuidex() will result in a linker error, since clang-cl sees the declaration for __cpuidex(), but then clang-cl ignores the pragma line, and then later it thinks the call is just a call to a function that isn't defined anywhere. This is not a problem: Just #include <intrin.h> instead of manually declaring the function. With clang-cl, intrin.h contains a definition of __cpuidex (and the other intrinsics), and no linker error will be emitted (and the definition is always_inline, so it's fast). There is just one wrinkle: Some system headers (e.g. windows.h) do manually declare intrinsics, mark them `#pragma intrinsic`, and then call them from other inline functions defined in system headers. So if some of our code calls one of these other inline functions, it needs intrin.h without us knowing about it. Luckily, we know only a single function where this is an issue in practice: SecureZeroMemory(), which calls __stosb. So we had to add explicit and mysterious includes for <intrin.h> to the two files that call that function. If more examples of this pop up, we can reevaluate if we want to stop force-including this header everywhere, but for now it seems overkill to inject this header into every translation unit just because two translation units need it. BUG=592745 ========== to ========== clang/win: Stop force-including intrin.h everywhere. MSVC and clang-cl offer a bunch of functions that are built-in to the compiler: Intrinsics. When an intrinsic is called, the compiler can emit specialized code for intrinsics. (It's free to emit a regular call to a function too though.) intrin.h includes prototypes (and with clang-cl, definitions) for intrinsics. MSVC also supports `#pragma intrinsics(name)`, which allows marking the intrinsic named "name" as something that should preferably be treated as an intrinsic (roughly ~inlined) instead of as a call. With this, it is possible to manually declare an intrinsic and then use that pragma, and since it'll be treated as an intrinsic, no linker error will happen. clang-cl doesn't yet implement `#pragma intrinsic`, so e.g. void __cpuidex(int CPUInfo[4], int info_type, int ecxvalue); #pragma intrinsic(__cpuidex) // later, call __cpuidex() will result in a linker error, since clang-cl sees the declaration for __cpuidex(), but then clang-cl ignores the pragma line, and then later it thinks the call is just a call to a function that isn't defined anywhere. This is not a problem: Just #include <intrin.h> instead of manually declaring the function. With clang-cl, intrin.h contains a definition of __cpuidex (and the other intrinsics), and no linker error will be emitted (and the definition is always_inline, so it's fast). There is just one wrinkle: Some system headers (e.g. windows.h) do manually declare intrinsics, mark them `#pragma intrinsic`, and then call them from other inline functions defined in system headers. So if some of our code calls one of these other inline functions, it needs intrin.h without us knowing about it. Luckily, we know only a single function where this is an issue in practice: SecureZeroMemory(), which calls __stosb. So we had to add explicit and mysterious includes for <intrin.h> to the two files that call that function. If more examples of this pop up, we can reevaluate if we want to force-include this header everywhere, but for now it seems overkill to inject this header into every translation unit just because two translation units need it. BUG=592745 ==========
done, thanks!
The CQ bit was checked by thakis@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2076483002/1
Message was sent while issue was closed.
Description was changed from ========== clang/win: Stop force-including intrin.h everywhere. MSVC and clang-cl offer a bunch of functions that are built-in to the compiler: Intrinsics. When an intrinsic is called, the compiler can emit specialized code for intrinsics. (It's free to emit a regular call to a function too though.) intrin.h includes prototypes (and with clang-cl, definitions) for intrinsics. MSVC also supports `#pragma intrinsics(name)`, which allows marking the intrinsic named "name" as something that should preferably be treated as an intrinsic (roughly ~inlined) instead of as a call. With this, it is possible to manually declare an intrinsic and then use that pragma, and since it'll be treated as an intrinsic, no linker error will happen. clang-cl doesn't yet implement `#pragma intrinsic`, so e.g. void __cpuidex(int CPUInfo[4], int info_type, int ecxvalue); #pragma intrinsic(__cpuidex) // later, call __cpuidex() will result in a linker error, since clang-cl sees the declaration for __cpuidex(), but then clang-cl ignores the pragma line, and then later it thinks the call is just a call to a function that isn't defined anywhere. This is not a problem: Just #include <intrin.h> instead of manually declaring the function. With clang-cl, intrin.h contains a definition of __cpuidex (and the other intrinsics), and no linker error will be emitted (and the definition is always_inline, so it's fast). There is just one wrinkle: Some system headers (e.g. windows.h) do manually declare intrinsics, mark them `#pragma intrinsic`, and then call them from other inline functions defined in system headers. So if some of our code calls one of these other inline functions, it needs intrin.h without us knowing about it. Luckily, we know only a single function where this is an issue in practice: SecureZeroMemory(), which calls __stosb. So we had to add explicit and mysterious includes for <intrin.h> to the two files that call that function. If more examples of this pop up, we can reevaluate if we want to force-include this header everywhere, but for now it seems overkill to inject this header into every translation unit just because two translation units need it. BUG=592745 ========== to ========== clang/win: Stop force-including intrin.h everywhere. MSVC and clang-cl offer a bunch of functions that are built-in to the compiler: Intrinsics. When an intrinsic is called, the compiler can emit specialized code for intrinsics. (It's free to emit a regular call to a function too though.) intrin.h includes prototypes (and with clang-cl, definitions) for intrinsics. MSVC also supports `#pragma intrinsics(name)`, which allows marking the intrinsic named "name" as something that should preferably be treated as an intrinsic (roughly ~inlined) instead of as a call. With this, it is possible to manually declare an intrinsic and then use that pragma, and since it'll be treated as an intrinsic, no linker error will happen. clang-cl doesn't yet implement `#pragma intrinsic`, so e.g. void __cpuidex(int CPUInfo[4], int info_type, int ecxvalue); #pragma intrinsic(__cpuidex) // later, call __cpuidex() will result in a linker error, since clang-cl sees the declaration for __cpuidex(), but then clang-cl ignores the pragma line, and then later it thinks the call is just a call to a function that isn't defined anywhere. This is not a problem: Just #include <intrin.h> instead of manually declaring the function. With clang-cl, intrin.h contains a definition of __cpuidex (and the other intrinsics), and no linker error will be emitted (and the definition is always_inline, so it's fast). There is just one wrinkle: Some system headers (e.g. windows.h) do manually declare intrinsics, mark them `#pragma intrinsic`, and then call them from other inline functions defined in system headers. So if some of our code calls one of these other inline functions, it needs intrin.h without us knowing about it. Luckily, we know only a single function where this is an issue in practice: SecureZeroMemory(), which calls __stosb. So we had to add explicit and mysterious includes for <intrin.h> to the two files that call that function. If more examples of this pop up, we can reevaluate if we want to force-include this header everywhere, but for now it seems overkill to inject this header into every translation unit just because two translation units need it. BUG=592745 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== clang/win: Stop force-including intrin.h everywhere. MSVC and clang-cl offer a bunch of functions that are built-in to the compiler: Intrinsics. When an intrinsic is called, the compiler can emit specialized code for intrinsics. (It's free to emit a regular call to a function too though.) intrin.h includes prototypes (and with clang-cl, definitions) for intrinsics. MSVC also supports `#pragma intrinsics(name)`, which allows marking the intrinsic named "name" as something that should preferably be treated as an intrinsic (roughly ~inlined) instead of as a call. With this, it is possible to manually declare an intrinsic and then use that pragma, and since it'll be treated as an intrinsic, no linker error will happen. clang-cl doesn't yet implement `#pragma intrinsic`, so e.g. void __cpuidex(int CPUInfo[4], int info_type, int ecxvalue); #pragma intrinsic(__cpuidex) // later, call __cpuidex() will result in a linker error, since clang-cl sees the declaration for __cpuidex(), but then clang-cl ignores the pragma line, and then later it thinks the call is just a call to a function that isn't defined anywhere. This is not a problem: Just #include <intrin.h> instead of manually declaring the function. With clang-cl, intrin.h contains a definition of __cpuidex (and the other intrinsics), and no linker error will be emitted (and the definition is always_inline, so it's fast). There is just one wrinkle: Some system headers (e.g. windows.h) do manually declare intrinsics, mark them `#pragma intrinsic`, and then call them from other inline functions defined in system headers. So if some of our code calls one of these other inline functions, it needs intrin.h without us knowing about it. Luckily, we know only a single function where this is an issue in practice: SecureZeroMemory(), which calls __stosb. So we had to add explicit and mysterious includes for <intrin.h> to the two files that call that function. If more examples of this pop up, we can reevaluate if we want to force-include this header everywhere, but for now it seems overkill to inject this header into every translation unit just because two translation units need it. BUG=592745 ========== to ========== clang/win: Stop force-including intrin.h everywhere. MSVC and clang-cl offer a bunch of functions that are built-in to the compiler: Intrinsics. When an intrinsic is called, the compiler can emit specialized code for intrinsics. (It's free to emit a regular call to a function too though.) intrin.h includes prototypes (and with clang-cl, definitions) for intrinsics. MSVC also supports `#pragma intrinsics(name)`, which allows marking the intrinsic named "name" as something that should preferably be treated as an intrinsic (roughly ~inlined) instead of as a call. With this, it is possible to manually declare an intrinsic and then use that pragma, and since it'll be treated as an intrinsic, no linker error will happen. clang-cl doesn't yet implement `#pragma intrinsic`, so e.g. void __cpuidex(int CPUInfo[4], int info_type, int ecxvalue); #pragma intrinsic(__cpuidex) // later, call __cpuidex() will result in a linker error, since clang-cl sees the declaration for __cpuidex(), but then clang-cl ignores the pragma line, and then later it thinks the call is just a call to a function that isn't defined anywhere. This is not a problem: Just #include <intrin.h> instead of manually declaring the function. With clang-cl, intrin.h contains a definition of __cpuidex (and the other intrinsics), and no linker error will be emitted (and the definition is always_inline, so it's fast). There is just one wrinkle: Some system headers (e.g. windows.h) do manually declare intrinsics, mark them `#pragma intrinsic`, and then call them from other inline functions defined in system headers. So if some of our code calls one of these other inline functions, it needs intrin.h without us knowing about it. Luckily, we know only a single function where this is an issue in practice: SecureZeroMemory(), which calls __stosb. So we had to add explicit and mysterious includes for <intrin.h> to the two files that call that function. If more examples of this pop up, we can reevaluate if we want to force-include this header everywhere, but for now it seems overkill to inject this header into every translation unit just because two translation units need it. BUG=592745 Committed: https://crrev.com/ac7670452f51ecfd1061749b3d5e15f8544a55ee Cr-Commit-Position: refs/heads/master@{#401613} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/ac7670452f51ecfd1061749b3d5e15f8544a55ee Cr-Commit-Position: refs/heads/master@{#401613} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
