diff --git a/Scripts/InstructionCountParser.py b/Scripts/InstructionCountParser.py index a8273ed50..c4b10ec1f 100755 --- a/Scripts/InstructionCountParser.py +++ b/Scripts/InstructionCountParser.py @@ -96,6 +96,7 @@ def GetHostFeatures(data): return HostFeaturesData def parse_json_data(json_filepath, json_filename, json_data, output_binary_path): + BinaryCacheVersion = 0 Bitness = 64 EnabledHostFeatures = HostFeatures.FEATURE_ANY DisabledHostFeatures = HostFeatures.FEATURE_ANY @@ -106,6 +107,9 @@ def parse_json_data(json_filepath, json_filename, json_data, output_binary_path) if ("Bitness" in items): Bitness = int(items["Bitness"]) + if ("BinaryCacheVersion" in items): + BinaryCacheVersion = int(items["BinaryCacheVersion"]) + if ("EnabledHostFeatures" in items): EnabledHostFeatures = GetHostFeatures(items["EnabledHostFeatures"]) @@ -177,6 +181,7 @@ def parse_json_data(json_filepath, json_filename, json_data, output_binary_path) # struct TestInfo; # struct DataHeader { # uint64_t Bitness; + # uint64_t BinaryCacheVersion; # uint64_t NumTests; # uint64_t EnabledHostFeatures; # uint64_t DisabledHostFeatures; @@ -197,6 +202,7 @@ def parse_json_data(json_filepath, json_filename, json_data, output_binary_path) # Add the header MemData += struct.pack('Q', Bitness) + MemData += struct.pack('Q', BinaryCacheVersion) MemData += struct.pack('Q', len(TestDataMap)) MemData += struct.pack('Q', EnabledHostFeatures.value) MemData += struct.pack('Q', DisabledHostFeatures.value) diff --git a/Scripts/UpdateInstructionCountJson.py b/Scripts/UpdateInstructionCountJson.py index 16bbffcc8..312c74644 100755 --- a/Scripts/UpdateInstructionCountJson.py +++ b/Scripts/UpdateInstructionCountJson.py @@ -10,7 +10,10 @@ def insert_before(d, key, item): items.insert(list(d.keys()).index(key), item) return dict(items) -def update_performance_numbers(performance_json_path, performance_json, new_json_numbers): +def update_performance_numbers(performance_json_path, performance_json, new_json_numbers, new_cache_version): + + performance_json["Features"]["BinaryCacheVersion"] = new_cache_version + for key, items in new_json_numbers.items(): if len(key) == 0: continue @@ -64,11 +67,19 @@ def main(): if not isinstance(performance_json_data, dict): raise TypeError('JSON data must be a dict') - new_json_numbers_data = json.loads(new_json_numbers_text) + new_json_numbers_base = json.loads(new_json_numbers_text) + + features = new_json_numbers_base["Features"] + if not isinstance(features, dict): + raise TypeError('features JSON data must be a dict') + + new_cache_version = features["BinaryCacheVersion"] + + new_json_numbers_data = new_json_numbers_base["Instructions"] if not isinstance(new_json_numbers_data, dict): raise TypeError('JSON data must be a dict') - return update_performance_numbers(performance_json_path, performance_json_data, new_json_numbers_data) + return update_performance_numbers(performance_json_path, performance_json_data, new_json_numbers_data, new_cache_version) except ValueError as ve: logging.error(f'JSON error: {ve}') return 1 diff --git a/Source/Tools/CodeSizeValidation/Main.cpp b/Source/Tools/CodeSizeValidation/Main.cpp index 75360042c..86de9e2d7 100644 --- a/Source/Tools/CodeSizeValidation/Main.cpp +++ b/Source/Tools/CodeSizeValidation/Main.cpp @@ -1,9 +1,10 @@ // SPDX-License-Identifier: MIT #include "DummyHandlers.h" #include "Common/HostFeatures.h" -#include "FEXCore/Core/Context.h" -#include "FEXCore/Debug/InternalThreadState.h" #include +#include +#include +#include #include #include #include @@ -12,6 +13,10 @@ #include +namespace FEXCore::DiskCache { +uint16_t GetFormatVersion(); +} + namespace CodeSize { class CodeSizeValidation final { public: @@ -224,6 +229,7 @@ struct TestInfo { struct TestHeader { uint64_t Bitness; + uint64_t BinaryCacheVersion; uint64_t NumTests {}; uint64_t EnabledHostFeatures; uint64_t DisabledHostFeatures; @@ -287,7 +293,34 @@ static bool TestInstructions(FEXCore::Context::Context* CTX, FEXCore::Core::Inte CurrentTest = reinterpret_cast(&CurrentTest->Code[CurrentTest->CodeSize]); } + auto ExpectedFormatVersion = FEXCore::DiskCache::GetFormatVersion(); + if (TestHeaderData->BinaryCacheVersion != ExpectedFormatVersion) { + // Disk cache binary version updated but failed to update instcount ci tracking. + LogMan::Msg::EFmt("Fail: TestHarness binary cache version is '{}' but test json is '{}'", ExpectedFormatVersion, + TestHeaderData->BinaryCacheVersion); + LogMan::Msg::EFmt("Fail: Please run `ninja instcountci_tests ; ninja instcountci_update_tests` and commit with `git commit -m " + "\"InstcountCI: Update\"` to update instcount CI files"); + TestsPassed = false; + } + if (UpdatedInstructionCountsPath) { + if (!TestsPassed && TestHeaderData->BinaryCacheVersion == ExpectedFormatVersion) { + // Binary cache versions matched but instructions mismatched. Need to update the format version. + // Print a message warning about this otherwise we'll forget about it. + LogMan::Msg::EFmt("Fail: Excuse me ma'am, sir, or other unworldly being that is running this software."); + LogMan::Msg::EFmt("Fail: InstcountCI results have changed but the FEXCore::DiskCache::FormatVersion hasn't been updated!"); + LogMan::Msg::EFmt("Fail: This means with your change you are invalidating disk cache entries for everyone. Be sure to know the " + "consequences!"); + LogMan::Msg::EFmt("Fail: Please increment that number, recompile everything and rerun `ninja instcountci_tests ; ninja " + "instcountci_update_tests`"); + LogMan::Msg::EFmt("Fail: DiskCache version should be incremented from '{}' to '{}'", ExpectedFormatVersion, ExpectedFormatVersion + 1); + + // Unlink the file, to ensure it doesn't update with `instcountci_update_tests` + unlink(UpdatedInstructionCountsPath); + + return TestsPassed; + } + // Unlink the file. unlink(UpdatedInstructionCountsPath); @@ -302,6 +335,12 @@ static bool TestInstructions(FEXCore::Context::Context* CTX, FEXCore::Core::Inte FD.Write("{\n", 2); + FD.Write(fextl::fmt::format("\t\"{}\": {{\n", "Features")); + FD.Write(fextl::fmt::format("\t\t\"{}\": {}\n", "BinaryCacheVersion", ExpectedFormatVersion)); + FD.Write(fextl::fmt::format("\t}},\n")); + + FD.Write(fextl::fmt::format("\t\"{}\": {{\n", "Instructions")); + CurrentTest = TestsStart; for (size_t i = 0; i < TestHeaderData->NumTests; ++i) { // Get the instruction stats. @@ -330,6 +369,8 @@ static bool TestInstructions(FEXCore::Context::Context* CTX, FEXCore::Core::Inte // Print a null member FD.Write(fextl::fmt::format("\t\"\": \"\"")); + FD.Write(fextl::fmt::format("\t}}\n")); + FD.Write("}\n", 2); } return TestsPassed;