Skip to content

Commit

Permalink
Add Windows/MSVC support (flashlight#1107)
Browse files Browse the repository at this point in the history
Summary:
Implement changes to support compilation with MSVC. This tests the Flashlight core with MSVC 2019; won't expand to apps/pkgs for now/will gate compilation given dependencies therein that aren't supported on Windows.

Required changes include:
- initialize the `flashlight` target after compiler options are set, and move those up. Eventually, this should probably be public properties of the `flashlight` target
- Add `\bigobj` as an MSVC compiler option since we aren't currently explicitly exporting symbols
- Don't bother checking for `-rdynamic` with windows because I don't want to support plugins for it anyways
- Introduce `FL_BUILD_PLUGIN` as a set option per the above to enforce the correct behavior
- Swap from `math.h` to `cmath` and compile with required `_USE_MATH_DEFINES` and `NOMINMAX` for `M_PI` and friends
- MSVC hates implicit overloads that cast from `std::string` to `std::filesystem::path` in argument lists (when used as refs) -- change the FL serialization API to rely on `fs::path` as the arg
- missing headers throughout
- some missing lambda captures that gcc/clang totally ignored
- MSVC 2019 doesn't allow aggregate initialization, sad
- don't check and optionally use `std::filesystem` linker flags with MSVC; no additional linking is required

Pull Request resolved: flashlight#1107

Test Plan: local compilation with MSVC 2019 on Windows 11, test with toy CI baselines

Reviewed By: bwasti

Differential Revision: D46080426

Pulled By: jacobkahn

fbshipit-source-id: f006e68aea9d798da72621e5d7538752e786d99b
  • Loading branch information
jacobkahn authored and facebook-github-bot committed May 22, 2023
1 parent 861502d commit f0e85e3
Show file tree
Hide file tree
Showing 21 changed files with 175 additions and 133 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,5 @@ wheels/

# Dev environment
.vscode
.vs
CMakeSettings.json
36 changes: 24 additions & 12 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@ list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CXX_STANDARD_REQUIRED ON)

# The CUDA standard is still C++14 to enable interopability with
# slightly older and still well-supported versions of CUDA/nvcc
# (e.g. CUDA < 11). This will be bumped to 17 once CUDA 11 is
# required.
set(CMAKE_CUDA_STANDARD 14)
set(CMAKE_CUDA_STANDARD_REQUIRED ON)

# Default directories for installation
set(FL_INSTALL_INC_DIR "include" CACHE PATH "Install path for headers")
set(FL_INSTALL_INC_DIR_HEADER_LOC ${FL_INSTALL_INC_DIR}/flashlight)
Expand All @@ -35,9 +42,23 @@ if(${COMPILER_SUPPORTS_RDYNAMIC})
set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -rdynamic")
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -rdynamic")
else()
message(WARNING
"This compiler doesn't support dynamic symbol exports. "
"Plugin functionality likely won't work.")
if (NOT WIN32) # Windows operates with explicit dll exports
message(WARNING
"This compiler doesn't support dynamic symbol exports. "
"Plugin functionality likely won't work.")
endif()
endif()
set(FL_BUILD_PLUGIN ${COMPILER_SUPPORTS_RDYNAMIC})

# ]------ The library target for the Flashlight core
add_library(flashlight)

# We haven't [yet] polluted Flashlight with explicit dllexport annotations
if (MSVC)
target_compile_options(flashlight PUBLIC $<$<NOT:$<COMPILE_LANGUAGE:CUDA>>:/bigobj>)
endif()
if (WIN32)
target_compile_definitions(flashlight PUBLIC _USE_MATH_DEFINES NOMINMAX)
endif()

include(InternalUtils)
Expand Down Expand Up @@ -73,16 +94,7 @@ option(FL_USE_CPU "Build CPU support for Flashlight backends" OFF)
option(FL_USE_OPENCL "Build OpenCL support for Flashlight backends" OFF)
option(FL_USE_MKL "Build MKL support for Flashlight backends" OFF)

# The CUDA standard is still C++14 to enable interopability with
# slightly older and still well-supported versions of CUDA/nvcc
# (e.g. CUDA < 11). This will be bumped to 17 once CUDA 11 is
# required.
set(CMAKE_CUDA_STANDARD 14)
set(CMAKE_CUDA_STANDARD_REQUIRED ON)

# --------------------------- Core ---------------------------
add_library(flashlight)

# Internal includes are implicitly defined as <flashlight...>
target_include_directories(
flashlight
Expand Down
26 changes: 21 additions & 5 deletions flashlight/fl/common/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,34 @@ target_sources(
${CMAKE_CURRENT_LIST_DIR}/DynamicBenchmark.cpp
${CMAKE_CURRENT_LIST_DIR}/Logging.cpp
${CMAKE_CURRENT_LIST_DIR}/Histogram.cpp
${CMAKE_CURRENT_LIST_DIR}/Plugin.cpp
${CMAKE_CURRENT_LIST_DIR}/Timer.cpp
)

if (${FL_BUILD_PLUGIN})
target_sources(flashlight PRIVATE ${CMAKE_CURRENT_LIST_DIR}/Plugin.cpp)
else()
message(WARNING "Compiler doesn't support -rdynamic - not building Plugin")
endif()

# A native threading library
find_package(Threads REQUIRED)
target_link_libraries(flashlight PUBLIC Threads::Threads)

# Dynamic lib loading needed for Plugins
target_link_libraries(flashlight PUBLIC ${CMAKE_DL_LIBS})

# std::filesystem -- remove this when requiring gcc >= 9
find_package(Filesystem REQUIRED COMPONENTS Final Experimental)
setup_install_find_module(${PROJECT_SOURCE_DIR}/cmake/FindFilesystem.cmake)
target_link_libraries(flashlight PUBLIC std::filesystem)
# To support __VA_ARGS__ for serialization macros
if (MSVC)
if (MSVC_VERSION LESS 1920)
target_compile_options(flashlight PUBLIC $<$<COMPILE_LANGUAGE:C,CXX>:/experimental:preprocessor>) # MSVC 2017
else()
target_compile_options(flashlight PUBLIC $<$<COMPILE_LANGUAGE:C,CXX>:/Zc:preprocessor>) # MSVC 2019+
endif()
endif()

# std::filesystem linking -- remove this when requiring gcc >= 9
if (NOT MSVC)
find_package(Filesystem REQUIRED COMPONENTS Final Experimental)
setup_install_find_module(${CMAKE_MODULE_PATH}/FindFilesystem.cmake)
target_link_libraries(flashlight PUBLIC std::filesystem)
endif()
8 changes: 5 additions & 3 deletions flashlight/fl/common/Serialization-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include <type_traits>
#include <utility>

#include "flashlight/fl/common/Filesystem.h"
#include "flashlight/fl/tensor/TensorBase.h"

#pragma once
Expand Down Expand Up @@ -128,7 +129,7 @@ serializeAs(T&& t, SaveConvFn saveConverter, LoadConvFn loadConverter) {
}

template <typename... Args>
void save(const std::string& filepath, const Args&... args) {
void save(const fs::path& filepath, const Args&... args) {
std::ofstream ofs(filepath, std::ios::binary);
save(ofs, args...);
}
Expand All @@ -140,7 +141,7 @@ void save(std::ostream& ostr, const Args&... args) {
}

template <typename... Args>
void load(const std::string& filepath, Args&... args) {
void load(const fs::path& filepath, Args&... args) {
std::ifstream ifs(filepath, std::ios::binary);
load(ifs, args...);
}
Expand Down Expand Up @@ -189,7 +190,8 @@ void save(

template <class Archive>
void load(Archive& ar, fl::Shape& dims, const uint32_t /* version */) {
// TODO{fl::Tensor} -- check version, then read dim4 into Shape (if version ==)
// TODO{fl::Tensor} -- check version, then read dim4 into Shape (if version
// ==)
std::vector<fl::Dim> vec;
ar(vec);
dims = fl::Shape(vec);
Expand Down
5 changes: 3 additions & 2 deletions flashlight/fl/common/Serialization.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include <iostream>
#include <type_traits>

#include "flashlight/fl/common/Filesystem.h"
#include "flashlight/fl/tensor/TensorBase.h"

#include <cereal/access.hpp>
Expand Down Expand Up @@ -54,7 +55,7 @@ namespace fl {
* @param args the objects to save (e.g. shared_ptr to Module)
*/
template <typename... Args>
void save(const std::string& filepath, const Args&... args);
void save(const fs::path& filepath, const Args&... args);

/**
* Save (serialize) the specified args to a binary file (via Cereal).
Expand All @@ -70,7 +71,7 @@ void save(std::ostream& ostr, const Args&... args);
* @param args the objects to load (expects default-constructed)
*/
template <typename... Args>
void load(const std::string& filepath, Args&... args);
void load(const fs::path& filepath, Args&... args);

/**
* Load (deserialize) the specified args from a binary file (via Cereal).
Expand Down
3 changes: 2 additions & 1 deletion flashlight/fl/contrib/modules/RawWavSpecAugment.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@
* LICENSE file in the root directory of this source tree.
*/

#include <math.h>
#include <cmath>
#include <sstream>
#include <stdexcept>
#include <numeric>

#include "flashlight/fl/common/Logging.h"
#include "flashlight/fl/contrib/modules/RawWavSpecAugment.h"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,11 @@

#include "flashlight/fl/contrib/modules/SinusoidalPositionEmbedding.h"

#include <math.h>
#include <cmath>
#include <stdexcept>
#include <algorithm>
#include <numeric>

#include "flashlight/fl/autograd/Functions.h"
#include "flashlight/fl/nn/Init.h"
#include "flashlight/fl/nn/Utils.h"
Expand Down
6 changes: 4 additions & 2 deletions flashlight/fl/dataset/FileBlobDataset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,22 @@
* LICENSE file in the root directory of this source tree.
*/

#include <stdexcept>

#include "flashlight/fl/dataset/FileBlobDataset.h"

namespace fl {

FileBlobDataset::FileBlobDataset(
const std::string& name,
const fs::path& name,
bool rw,
bool truncate)
: name_(name) {
mode_ = (rw ? std::ios_base::in | std::ios_base::out : std::ios_base::in);
{
std::ofstream fs(name_, (truncate ? mode_ | std::ios_base::trunc : mode_));
if (!fs.is_open()) {
throw std::runtime_error("could not open file " + name);
throw std::runtime_error("could not open file " + name.string());
}
}
readIndex();
Expand Down
5 changes: 3 additions & 2 deletions flashlight/fl/dataset/FileBlobDataset.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#pragma once

#include "flashlight/fl/common/Filesystem.h"
#include "flashlight/fl/dataset/BlobDataset.h"

#include <fstream>
Expand All @@ -33,7 +34,7 @@ class FileBlobDataset : public BlobDataset {
* already exists.
*/
explicit FileBlobDataset(
const std::string& name,
const fs::path& name,
bool rw = false,
bool truncate = false);

Expand All @@ -47,7 +48,7 @@ class FileBlobDataset : public BlobDataset {
bool isEmptyData() const override;

private:
std::string name_;
fs::path name_;
std::ios_base::openmode mode_;
std::shared_ptr<std::fstream> getStream() const;

Expand Down
1 change: 1 addition & 0 deletions flashlight/fl/dataset/ShuffleDataset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "flashlight/fl/dataset/ShuffleDataset.h"

#include <algorithm>
#include <numeric>

namespace fl {

Expand Down
2 changes: 1 addition & 1 deletion flashlight/fl/examples/RnnLm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ int main(int argc, char** argv) {

SGDOptimizer opt(model.params(), learning_rate);

auto eval_loop = [&model, &criterion](LMDataset& dataset) {
auto eval_loop = [&model, &criterion, kInputIdx, kTargetIdx](LMDataset& dataset) {
AverageValueMeter avg_loss_meter;
Variable output, h, c;
for (auto& example : dataset) {
Expand Down
1 change: 1 addition & 0 deletions flashlight/fl/tensor/Index.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#pragma once

#include <variant>
#include <optional>

#include "flashlight/fl/tensor/TensorBase.h"

Expand Down
3 changes: 0 additions & 3 deletions flashlight/fl/tensor/Init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@

#include <mutex>

#include <iostream>
#include <string>

#include "flashlight/fl/tensor/DefaultTensorType.h"
#include "flashlight/fl/tensor/TensorBackend.h"

Expand Down
1 change: 1 addition & 0 deletions flashlight/fl/tensor/TensorBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include <stdexcept>
#include <utility>
#include <algorithm>

#include "flashlight/fl/tensor/DefaultTensorType.h"
#include "flashlight/fl/tensor/TensorAdapter.h"
Expand Down
2 changes: 2 additions & 0 deletions flashlight/fl/tensor/backend/af/ArrayFireReductions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

#include "flashlight/fl/tensor/backend/af/ArrayFireBackend.h"

#include <algorithm>

#include <af/arith.h>
#include <af/gfor.h>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ struct BinopInfo {
};

struct OneDnnOpFusion::SearchState {
SearchState(Node* root, std::vector<BinopInfo> binopInfos)
: searchRoot(root), accumulatedBinopInfos(binopInfos) {}
Node* searchRoot;
// Assume `searchRoot == binop2`
//
Expand Down Expand Up @@ -94,7 +96,7 @@ bool shouldNodeBeFused(const Node* node) {
} // namespace

Node* OneDnnOpFusion::rewriteFrom(Node* node) {
SearchState state{.searchRoot = node, .accumulatedBinopInfos = {}};
SearchState state(node, /* accumulatedBinopInfos = */ {});
auto fusedNode = searchAndFuse(node, state);
node->replaceAllUsesWith(fusedNode);
return fusedNode;
Expand Down
Loading

0 comments on commit f0e85e3

Please sign in to comment.