Better MSVC support

* Avoiding use of templated variables.
* Now compling on MSVC with /WX (warnings as errors).
* Fixed all MSVC warnings.
* Fixed MSVC source_groups.

CMakeLists.txt

@@ -11,9 +11,25 @@ if (NOT MSVC)
         add_compile_options(-msse4.1)
     endif()
 else()
-    add_compile_options(/W3 /MP /Zi /Zo /EHsc)
+    add_compile_options(/W3 /MP /Zi /Zo /EHsc /WX)
 endif()
 
+# This function should be passed a list of all files in a target. It will automatically generate
+# file groups following the directory hierarchy, so that the layout of the files in IDEs matches the
+# one in the filesystem.
+function(create_directory_groups)
+    # Place any files that aren't in the source list in a separate group so that they don't get in
+    # the way.
+    source_group("Other Files" REGULAR_EXPRESSION ".")
+
+    foreach(file_name ${ARGV})
+        get_filename_component(dir_name "${file_name}" PATH)
+        # Group names use '\' as a separator even though the entire rest of CMake uses '/'...
+        string(REPLACE "/" "\\" group_name "${dir_name}")
+        source_group("${group_name}" FILES "${file_name}")
+    endforeach()
+endfunction()
+
 # Arch detection
 include(CheckSymbolExists)
 function(detect_architecture symbol arch)

src/CMakeLists.txt

@@ -48,6 +48,6 @@ set(HEADERS
     interface/interface.h
     )
 
-source_group(dynarmic FILES ${SRCS} ${HEADERS})
+create_directory_groups(${SRCS} ${HEADERS})
 add_library(dynarmic STATIC ${SRCS} ${HEADERS})
 set_target_properties(dynarmic PROPERTIES LINKER_LANGUAGE CXX)

src/backend_x64/reg_alloc.cpp

@@ -37,7 +37,7 @@ static Gen::OpArg SpillToOpArg(HostLoc loc) {
     ASSERT(HostLocIsSpill(loc));
 
     size_t i = static_cast<size_t>(loc) - static_cast<size_t>(HostLoc::FirstSpill);
-    return Gen::MDisp(Gen::R15, offsetof(JitState, Spill) + i * sizeof(u32));
+    return Gen::MDisp(Gen::R15, static_cast<int>(offsetof(JitState, Spill) + i * sizeof(u32)));
 }
 
 Gen::X64Reg RegAlloc::DefRegister(IR::Value* def_value, std::initializer_list<HostLoc> desired_locations) {

src/backend_x64/routines.cpp

@@ -22,7 +22,8 @@ Routines::Routines() {
 }
 
 size_t Routines::RunCode(JitState* jit_state, CodePtr basic_block, size_t cycles_to_run) const {
-    ASSERT(cycles_to_run <= std::numeric_limits<decltype(jit_state->cycles_remaining)>::max());
+    constexpr size_t max_cycles_to_run = static_cast<size_t>(std::numeric_limits<decltype(jit_state->cycles_remaining)>::max());
+    ASSERT(cycles_to_run <= max_cycles_to_run);
 
     jit_state->cycles_remaining = cycles_to_run;
     run_code(jit_state, basic_block);

src/common/bit_util.h

@@ -10,6 +10,11 @@
 
 #include "common/common_types.h"
 
+#ifdef _MSC_VER
+#pragma warning(push)
+#pragma warning(disable:4554)
+#endif
+
 namespace Dynarmic {
 namespace Common {
 
@@ -35,7 +40,7 @@ template<size_t bit_position, typename T>
 constexpr bool Bit(const T value) {
     static_assert(bit_position < BitSize<T>(), "bit_position must be smaller than size of T");
 
-    return (value >> bit_position) & 1;
+    return ((value >> bit_position) & 1) != 0;
 }
 
 /// Sign-extends a value that has NBits bits to the full bitwidth of type T.
@@ -53,3 +58,7 @@ inline T SignExtend(const T value) {
 
 } // namespace Common
 } // namespace Dynarmic
+
+#ifdef _MSC_VER
+#pragma warning(pop)
+#endif

src/common/x64/cpu_detect.cpp

@@ -82,7 +82,7 @@ static CPUCaps Detect() {
         caps.vendor = CPUVendor::OTHER;
 
     // Set reasonable default brand string even if brand string not available
-    strcpy(caps.cpu_string, caps.brand_string);
+    strcpy_s(caps.cpu_string, caps.brand_string);
 
     // Detect family and other miscellaneous features
     if (max_std_fn >= 1) {

src/frontend/decoder/arm.h

@@ -9,9 +9,8 @@
 #pragma once
 
 #include <algorithm>
-#include <array>
 #include <functional>
-#include <tuple>
+#include <vector>
 
 #include <boost/optional.hpp>
 
@@ -59,7 +58,8 @@ private:
 };
 
 template<typename V>
-static const std::array<ArmMatcher<V>, 6> g_arm_instruction_table = {
+boost::optional<const ArmMatcher<V>&> DecodeArm(u32 instruction) {
+    const static std::vector<ArmMatcher<V>> table = {
 
 #define INST(fn, name, bitstring) detail::detail<ArmMatcher, u32, 32>::GetMatcher<decltype(fn), fn>(name, bitstring)
 
@@ -338,13 +338,10 @@ static const std::array<ArmMatcher<V>, 6> g_arm_instruction_table = {
 
     };
 
-template<typename Visitor>
-boost::optional<const ArmMatcher<Visitor>&> DecodeArm(u32 instruction) {
-    const auto& table = g_arm_instruction_table<Visitor>;
     const auto matches_instruction = [instruction](const auto& matcher) { return matcher.Matches(instruction); };
 
     auto iter = std::find_if(table.begin(), table.end(), matches_instruction);
-    return iter != table.end() ? boost::make_optional<const ArmMatcher<Visitor>&>(*iter) : boost::none;
+    return iter != table.end() ? boost::make_optional<const ArmMatcher<V>&>(*iter) : boost::none;
 }
 
 } // namespace Arm

src/frontend/decoder/decoder_detail.h

@@ -98,6 +98,10 @@ private:
     template<typename FnT, FnT fn>
     struct VisitorCaller;
 
+#ifdef _MSC_VER
+#pragma warning(push)
+#pragma warning(disable:4800)
+#endif
     template<typename Visitor, typename ...Args, typename CallRetT, CallRetT (Visitor::*fn)(Args...)>
     struct VisitorCaller<CallRetT(Visitor::*)(Args...), fn> {
         template<size_t ...iota>
@@ -110,6 +114,9 @@ private:
             };
         }
     };
+#ifdef _MSC_VER
+#pragma warning(pop)
+#endif
 
 public:
     /**

src/frontend/decoder/thumb16.h

@@ -8,7 +8,7 @@
 
 #include <array>
 #include <functional>
-#include <tuple>
+#include <vector>
 
 #include <boost/optional.hpp>
 
@@ -56,7 +56,8 @@ private:
 };
 
 template<typename V>
-const std::array<Thumb16Matcher<V>, 44> g_thumb16_instruction_table = {
+boost::optional<const Thumb16Matcher<V>&> DecodeThumb16(u16 instruction) {
+    const static std::vector<Thumb16Matcher<V>> table = {
 
 #define INST(fn, name, bitstring) detail::detail<Thumb16Matcher, u16, 16>::GetMatcher<decltype(fn), fn>(name, bitstring)
 
@@ -151,15 +152,12 @@ const std::array<Thumb16Matcher<V>, 44> g_thumb16_instruction_table = {
 
     };
 
-template<typename Visitor>
+    const auto matches_instruction = [instruction](const auto& matcher){ return matcher.Matches(instruction); };
-boost::optional<const Thumb16Matcher<Visitor>&> DecodeThumb16(u16 instruction) {
-    const auto& table = g_thumb16_instruction_table<Visitor>;
-    auto matches_instruction = [instruction](const auto& matcher){ return matcher.Matches(instruction); };
 
     assert(std::count_if(table.begin(), table.end(), matches_instruction) <= 1);
 
     auto iter = std::find_if(table.begin(), table.end(), matches_instruction);
-    return iter != table.end() ? boost::make_optional<const Thumb16Matcher<Visitor>&>(*iter) : boost::none;
+    return iter != table.end() ? boost::make_optional<const Thumb16Matcher<V>&>(*iter) : boost::none;
 }
 
 } // namespace Arm

src/frontend/ir/ir.h

@@ -81,7 +81,7 @@ public:
 protected:
     friend class Inst;
 
-    Value(Opcode op_) : op(op_) {}
+    explicit Value(Opcode op_) : op(op_) {}
 
     void AddUse(ValuePtr owner);
     void RemoveUse(ValuePtr owner);
@@ -98,7 +98,7 @@ private:
     };
     std::list<Use> uses;
 
-    intptr_t tag;
+    intptr_t tag = 0;
 };
 
 /// Representation of a u1 immediate.

src/frontend/translate/translate_arm.cpp

@@ -80,13 +80,13 @@ struct ArmTranslatorVisitor final {
         return true;
     }
 
-    u32 rotr(u32 x, int shift) {
+    static u32 rotr(u32 x, int shift) {
         shift &= 31;
         if (!shift) return x;
         return (x >> shift) | (x << (32 - shift));
     }
 
-    u32 ArmExpandImm(int rotate, Imm8 imm8) {
+    static u32 ArmExpandImm(int rotate, Imm8 imm8) {
         return rotr(static_cast<u32>(imm8), rotate*2);
     }
 

src/frontend/translate/translate_thumb.cpp

@@ -527,7 +527,7 @@ enum class ThumbInstSize {
     Thumb16, Thumb32
 };
 
-static std::tuple<u32, ThumbInstSize> ReadThumbInstruction(u32 arm_pc, MemoryRead32FuncType memory_read_32) {
+std::tuple<u32, ThumbInstSize> ReadThumbInstruction(u32 arm_pc, MemoryRead32FuncType memory_read_32) {
     u32 first_part = (*memory_read_32)(arm_pc & 0xFFFFFFFC);
     if ((arm_pc & 0x2) != 0)
         first_part >>= 16;

tests/CMakeLists.txt

@@ -31,7 +31,7 @@ set(HEADERS
     skyeye_interpreter/skyeye_common/vfp/vfp_helper.h
     )
 
-source_group(dynarmic_tests FILES ${SRCS} ${HEADERS})
+create_directory_groups(${SRCS} ${HEADERS})
 add_executable(dynarmic_tests ${SRCS})
 target_link_libraries(dynarmic_tests dynarmic)
 set_target_properties(dynarmic_tests PROPERTIES LINKER_LANGUAGE CXX)

tests/arm/fuzz_arm.cpp

@@ -191,7 +191,7 @@ void FuzzJitArm(const size_t instruction_count, const size_t instructions_to_exe
 
         // Run interpreter
         write_records.clear();
-        interp.NumInstrsToExecute = instructions_to_execute_count;
+        interp.NumInstrsToExecute = static_cast<unsigned>(instructions_to_execute_count);
         InterpreterMainLoop(&interp);
         auto interp_write_records = write_records;
         {
@@ -201,7 +201,7 @@ void FuzzJitArm(const size_t instruction_count, const size_t instructions_to_exe
 
         // Run jit
         write_records.clear();
-        jit.Run(instructions_to_execute_count);
+        jit.Run(static_cast<unsigned>(instructions_to_execute_count));
         auto jit_write_records = write_records;
 
         // Compare

tests/arm/fuzz_thumb.cpp

@@ -189,7 +189,7 @@ void FuzzJitThumb(const size_t instruction_count, const size_t instructions_to_e
 
         // Run interpreter
         write_records.clear();
-        interp.NumInstrsToExecute = instructions_to_execute_count;
+        interp.NumInstrsToExecute = static_cast<unsigned>(instructions_to_execute_count);
         InterpreterMainLoop(&interp);
         auto interp_write_records = write_records;
         {
@@ -199,7 +199,7 @@ void FuzzJitThumb(const size_t instruction_count, const size_t instructions_to_e
 
         // Run jit
         write_records.clear();
-        jit.Run(instructions_to_execute_count);
+        jit.Run(static_cast<unsigned>(instructions_to_execute_count));
         auto jit_write_records = write_records;
 
         // Compare

tests/skyeye_interpreter/dyncom/arm_dyncom_interpreter.cpp

@@ -662,6 +662,9 @@ static void LnSWoUB(ScaledRegisterOffset)(ARMul_State* cpu, unsigned int inst, u
     virt_addr = addr;
 }
 
+#ifdef _MSC_VER
+#pragma warning(disable:4200)
+#endif
 struct arm_inst {
     unsigned int idx;
     unsigned int cond;

tests/skyeye_interpreter/skyeye_common/armstate.cpp

@@ -10,7 +10,7 @@
 namespace Common {
 inline u16 swap16(u16 data) {return (data >> 8) | (data << 8);}
 inline u32 swap32(u32 data) {return (swap16(data) << 16) | swap16(data >> 16);}
-inline u64 swap64(u64 data) {return ((u64)swap32(data) << 32) | swap32(data >> 32);}
+inline u64 swap64(u64 data) {return ((u64)swap32((u32)data) << 32) | (u64)swap32(data >> 32);}
 }
 
 ARMul_State::ARMul_State(PrivilegeMode initial_mode)