Skip to content

compiler.h changes for Windows MSVC support #6623

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 5, 2024
Merged

compiler.h changes for Windows MSVC support #6623

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion kernels/prim_ops/register_prim_ops.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ static Kernel prim_ops[] = {
EValue& out = *stack[2];
if (a.isInt() && b.isInt()) {
const int64_t quot = a.toInt() / b.toInt();
if (std::signbit(a.toInt()) == std::signbit(b.toInt())) {
if ((a.toInt() < 0) == (b.toInt() < 0)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels unrelated to PR in question, is it?

out = EValue(quot);
return;
}
Expand Down
2 changes: 0 additions & 2 deletions runtime/executor/program.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@
#define ET_ENABLE_PROGRAM_VERIFICATION 1
#endif

#pragma clang diagnostic ignored "-Wshadow"

namespace executorch {
namespace runtime {

Expand Down
26 changes: 21 additions & 5 deletions runtime/platform/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
#error "You need C++17 to compile ExecuTorch"
#endif

#if defined(_WIN32) && (defined(min) || defined(max))
#if defined(_MSC_VER) && (defined(min) || defined(max))
#error \
"Macro clash with min and max -- define NOMINMAX when compiling your program on Windows"
#endif
Expand Down Expand Up @@ -100,22 +100,38 @@
#endif // (__cplusplus) >= 202002L

/// Define a C symbol with weak linkage.
#ifdef _MSC_VER
// There currently doesn't seem to be a great way to do this in Windows and
// given that weak linkage is not really critical on Windows, we'll just leave
// it as a stub.
#define ET_WEAK
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, we are adding Windows x64 Build Support #6979. I use .\install_requirements.bat --pybind xnnpack to build on windows. And we encountered duplicate symbol problem.
image
I investigated and found that windows didn't support weak symbol. Do you have some insights how to resolve this?

#else
#define ET_WEAK __attribute__((weak))
#endif

/**
* Annotation marking a function as printf-like, providing compiler support
* for format string argument checking.
*/
#ifdef _MSC_VER
#include <sal.h>
#define ET_PRINTFLIKE(_string_index, _va_index) _Printf_format_string_
#else
#define ET_PRINTFLIKE(_string_index, _va_index) \
__attribute__((format(printf, _string_index, _va_index)))

/// Name of the source file without a directory string.
#define ET_SHORT_FILENAME (__builtin_strrchr("/" __FILE__, '/') + 1)
#endif

#ifndef __has_builtin
#define __has_builtin(x) (0)
#endif

#if __has_builtin(__builtin_strrchr)
/// Name of the source file without a directory string.
#define ET_SHORT_FILENAME (__builtin_strrchr("/" __FILE__, '/') + 1)
#else
#define ET_SHORT_FILENAME __FILE__
#endif

#if __has_builtin(__builtin_LINE)
/// Current line as an integer.
#define ET_LINE __builtin_LINE()
Expand All @@ -141,7 +157,7 @@
#endif // ifndef

// Define size_t and ssize_t.
#ifndef _WIN32
#ifndef _MSC_VER
#include <sys/types.h>
#else
#include <stddef.h>
Expand Down
Loading