fix: keep scalar bitwise src/dst in distinct storage#631
Open
HecreReed wants to merge 3 commits intohw-native-sys:mainfrom
Open
fix: keep scalar bitwise src/dst in distinct storage#631HecreReed wants to merge 3 commits intohw-native-sys:mainfrom
HecreReed wants to merge 3 commits intohw-native-sys:mainfrom
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 introduces logic to ensure that scalar bitwise operations (TAndS, TOrS, TXorS) use distinct local storage for their source and destination operands. This is implemented by adding a static analysis helper to trace tile storage and updating the memory planning pass to record semantic conflicts between these operands. A review comment identifies a critical type mismatch in the IR verification logic where an attribute is passed to a function expecting a value, which would result in a compilation error.
| auto addressSpace = getPTOMemorySpaceEnum(allocTile.getResult().getType()); | ||
| if (!isLocalStorageSpace(addressSpace) || !allocTile.getAddr()) | ||
| return std::nullopt; | ||
| auto addr = getConstantIntegerValue(allocTile.getAddr()); |
There was a problem hiding this comment.
The getConstantIntegerValue function expects a Value as an argument, but allocTile.getAddr() returns an IntegerAttr (attribute). This will cause a compilation error. Please use the appropriate way to retrieve the integer value from the attribute, for example, allocTile.getAddr().getInt().
Codex Review该评论由 review 机器人自动更新。
SummaryPR #631 introduces a deterministic sample/CI regression by forcing the mgather/mscatter Python samples to emit A5-targeted modules. Findings
Adding |
Collaborator
Author
|
/run a5 |
|
已接收
页面会自动刷新，可以直接看当前阶段、排队情况和最近结果。 |
A5 板测失败
日志尾部 |
Collaborator
Author
|
/run a5 |
|
已接收
页面会自动刷新，可以直接看当前阶段、排队情况和最近结果。 |
A5 板测失败
失败用例
|
A5 板测失败详情：PR #631quant
quant_asym
partarg
mscatter
mgather
abs
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
pto.tands,pto.tors, andpto.txorsDetails
This follows issue #614 direction 2: keep the ISA/frontend distinct-storage constraint and make PTOAS planning honor it.
PlanMemory already had semantic conflict tracking for scratch-vs-dst style cases, but it did not explicitly model the
src/dststorage constraint for scalar bitwise ops. That allowed memory planning to potentially co-locate those buffers if liveness alone looked reusable.The verifier side also only rejected
src == dst, which missed aliasing forms such as zero-offset subviews of the same statically addressed local tile.Validation
git diff --checklib/PTO/IR/PTO.cpp.olib/PTO/Transforms/PTOPlanMemory.cpp.optoasbuild is currently blocked by pre-existing unrelated-Werrorfailures inPTOToEmitC.cpp,PTOTypeDefs.cpp, andPTOSyncUtils.cpp