[future] 1904-generate_floorplanningpy-and-floorplanning-ui-must-use-configjson-instead-of-vprxml#173
Conversation
…how_arch_resources when config.json is invalid
…n and device change
|
|
||
| // Load configFile and return the set of missing required keys. On a load/parse | ||
| // failure returns false and sets error (the keys set is then meaningless). | ||
| bool checkConfig(const std::filesystem::path& configFile, |
There was a problem hiding this comment.
checkConfig() always fails for encrypted files
FileUtils::GetFileContent(configFile) - raw ciphertext - json::parse() throws. checkConfig() returns false for every encrypted device unconditionally. The VPR fallback runs on every single GUI load even when config.json.en is perfectly valid. The fast path is completely broken for encrypted devices.
Fix needed: Replace the raw read with QLDeviceManager::getInstance()->loadDeviceConfigJSON() which already handles the full decrypt-in-memory pipeline.
| constexpr int kFirstCoreColumn = 1; | ||
|
|
||
| // The keys DeviceGridDescriptor / generate_floorplanning need. | ||
| const std::vector<std::string>& requiredKeys() { |
There was a problem hiding this comment.
requiredKeys() references DSP_SIZE/BRAM_SIZE which don't exist in any current config.json
Even if fix #1 is applied, checkConfig() will still find DSP_SIZE and BRAM_SIZE missing - because no config.json in device_data/ has those keys yet. Fast path still never fires. VPR fallback still always runs.
Fix needed: Either add DSP_SIZE/BRAM_SIZE to the device_data source config.json files, or remove them from requiredKeys().
There was a problem hiding this comment.
DSP_SIZE/BRAM_SIZE are already part of config.json. they were recently added. At least i see them for GF12nm 3030 device.
If we remove them fromr equired that we have to use hardcoded sizes.
|
|
||
| } // namespace | ||
|
|
||
| std::filesystem::path FloorplanningConfigProvider::getEffectiveConfig() { |
There was a problem hiding this comment.
Raw file read breaks once checkConfig() is fixed
const QByteArray content = QByteArray::fromStdString(
FOEDAG::FileUtils::GetFileContent(deviceConfigFile));
Currently getEffectiveConfig() never returns a .en path because checkConfig() is broken (bug above). Once that is fixed, getEffectiveConfig() will return config.json.en for encrypted devices, and this GetFileContent() call will also read ciphertext - QJsonDocument::fromJson() fails - floorplanning cannot start.
getEffectiveConfig() should guarantee it never returns a .en path to callers -decrypt to a temporary plaintext file first, then return that path.
There was a problem hiding this comment.
yes, i agree, i didn't aware that config.json will be under encryption, need review this
| #endif // DISABLE_COMPILER_TEMP_FILES_GUARD_WORKAROUND | ||
|
|
||
| BaseVprCommand(); // we need this call in order to decrypt arch file and store it as m_architectureFile | ||
| std::filesystem::path CompilerOpenFPGA_ql::deviceConfigFilePath(const QLDeviceTarget& device) const |
There was a problem hiding this comment.
deviceConfigFilePath() hardcodes "config.json", misses encrypted variant
std::filesystem::path CompilerOpenFPGA_ql::deviceConfigFilePath(const QLDeviceTarget& device) const {
return QLDeviceManager::getInstance()->deviceTypeDirPath(device) / std::string("config.json");
}
For encrypted devices only config.json.en exists - config.json does not. So FileUtils::FileExists() on this path returns false, and bram_type reading is silently skipped. Should use QLDeviceManager::getInstance()->deviceConfigJSONPath(device) instead, which already handles both variants.
| args.push_back("--fpga_layout"); | ||
| args.push_back(QLSettingsManager::getStringValue("general", "device", "layout")); | ||
| args.push_back("--device_config_file"); | ||
| args.push_back(FloorplanningConfigProvider::getEffectiveConfig().string()); |
There was a problem hiding this comment.
No empty-path guard before pushing to args
args.push_back(FloorplanningConfigProvider::getEffectiveConfig().string());
main_window_ql.cpp correctly guards against an empty return from getEffectiveConfig(). This call site does not - if both the config check and VPR fallback fail, an empty string is pushed as a generate_floorplanning.py argument and the error surfaces only as a cryptic script failure. Add the same empty-path check here.
w0lek
changed the title
2 participants
No description provided.