-
Notifications
You must be signed in to change notification settings - Fork 29
[AIE2P] split offsets before postincrment in large Load/Stores #675
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: aie-public
Are you sure you want to change the base?
Conversation
case AIE2P::G_AIE_POSTINC_3D_STORE: { | ||
for (unsigned SubRegIdx = 0; SubRegIdx < SplitFactor; ++SubRegIdx) { | ||
for (unsigned Part = 0; Part < SplitFactor; ++Part) { | ||
const unsigned SubRegIdx = AMI.MemI.getOpcode() == AIE2P::G_STORE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to handle differently S_STORE
? They will expand to offset stores anyway... If we have a strong objection we could also have a comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch, we can simplify it
case AIE2P::G_AIE_POSTINC_2D_LOAD: | ||
case AIE2P::G_AIE_POSTINC_3D_LOAD: { | ||
for (unsigned SubRegIdx = 0; SubRegIdx < SplitFactor; ++SubRegIdx) { | ||
for (unsigned Part = 0; Part < SplitFactor; ++Part) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as the G_STORE
one.
3d93257
to
d2025b0
Compare
Do you think we could have another QoR run because of the G_LOAD/G_STORE change? |
Before force pushing the change, I ran QoR to verify nothing degraded.
|
This looks good to me, I only recommend to fix the commit message before approval. |
Put the zero-offset part last, since it doesn't need the pointer to survive.
d2025b0
to
1817126
Compare
I updated the commit message |
Put the zero-offset part last, since it doesn't need the pointer to survive.
Note: This is a preparation PR for GEMM.