fix(ir): make parser arch context-scoped and add issue487 lit regression#490
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request transitions the PTO parser's target architecture tracking from a thread-local variable to a context-aware mechanism using a global map and mutex. This change allows different MLIRContext instances to maintain their own architecture settings. A new test case was added to verify the parser-scope architecture fallback. Feedback suggests replacing the global map and mutex with an MLIRContext extension to avoid synchronization bottlenecks and potential memory leaks, aligning better with MLIR's architecture.
| std::mutex parserTargetArchMutex; | ||
| std::unordered_map<const MLIRContext *, PTOParserTargetArch> | ||
| parserTargetArchByContext; |
There was a problem hiding this comment.
Using a global map and mutex to store context-scoped state is generally discouraged in MLIR. It introduces a global synchronization bottleneck that can impact performance during parallel verification and risks memory leaks if MLIRContext objects are destroyed without explicitly clearing the map.
A more idiomatic and efficient approach is to use an MLIRContext extension. Extensions are owned by the context, ensuring their lifetime is correctly managed, and they avoid global locks for lookup.
Example implementation:
class PTOParserArchExtension : public MLIRContext::Extension {
public:
using Extension::Extension;
PTOParserTargetArch arch = PTOParserTargetArch::Unspecified;
};
// In get/set functions:
context->getOrCreateExtension<PTOParserArchExtension>().arch = arch;
Codex Review该评论由 review 机器人自动更新。
Summary未检查到 PR #490 存在问题 FindingsNo issues found. |
A3 板测完成（有跳过）
|
3 participants
Summary
This PR fixes parser-arch propagation for verifier fallback by replacing TLS parser-arch state with MLIRContext-scoped state.
thread_localparser target arch storage withMLIRContext* -> PTOParserTargetArchmap guarded by mutex.op->getContext().ptoasparse scope helper usage to pass&context.Why
getVerifierTargetArch()can run during parse-time verification, and verifier may processIsolatedFromAbovechildren in parallel. TLS parser-arch is not shared across worker threads, which can cause A5 inputs to be mis-verified as A2/A3.Using context-scoped arch state makes parser fallback deterministic across threads for the same MLIRContext.
Test
cmake --build build --target ptoas -j 8build/tools/ptoas/ptoas --pto-arch=a5 test/basic/issue487_tsel_i8_parser_arch_scope.pto -o - 2>&1 | FileCheck test/basic/issue487_tsel_i8_parser_arch_scope.pto --check-prefix=A5build/tools/ptoas/ptoas --pto-arch=a3 test/basic/issue487_tsel_i8_parser_arch_scope.pto -o - 2>&1 | FileCheck test/basic/issue487_tsel_i8_parser_arch_scope.pto --check-prefix=A3Closes #487