Skip to content
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

Fix conv_fill_spec_pad.mlir #706

Merged
merged 5 commits into from
Aug 27, 2024
Merged

Fix conv_fill_spec_pad.mlir #706

merged 5 commits into from
Aug 27, 2024

Conversation

yzhang93
Copy link
Contributor

This change will get rid of the warning conv_fill_spec_pad.mlir:57:3: warning: op argument #0 is not consumed in the body but is marked as consumed.

Also attempt to fix #704.

Copy link
Contributor

@newling newling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Seems like the conv_fill_spec_pad problem is solved.

@yzhang93
Copy link
Contributor Author

Thanks! Seems like the conv_fill_spec_pad problem is solved.

Well, I still see some failures for this test https://github.com/nod-ai/iree-amd-aie/actions/runs/10567875494/job/29277762727?pr=706#step:9:11037. But then it passed after repeating. So I guess now it doesn't consistently fail but only flaky.

@newling
Copy link
Contributor

newling commented Aug 26, 2024

Ah ok I thought this one failed consistently on OSX and didn't have 5 chances to pass

Copy link
Collaborator

@makslevental makslevental left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not familiar with these transform schedules but looks fine to me.

Comment on lines 186 to 190
// %conv_pre_contract =
// transform.structured.match ops{["linalg.conv_1d_ncw_fcw"]}
// in %fused_for_all : (!any) -> !any

transform.structured.vectorize %conv_pre_contract : !any
// transform.structured.vectorize %conv_pre_contract : !any
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason to save this as a comment rather than just remove?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh, I was just trying to see if this part of the codes caused flaky crash, but seems not. Without these, it still have some failures. So I'm going to keep blocking this test for MacOS Ci until figuring out the reason.

Is there a MacOS machine that I can use to debug the CI failure?

@makslevental
Copy link
Collaborator

makslevental commented Aug 27, 2024

Q: why not just get rid of it? Personally I'm not a fan of transform schedules but that's just me 🤷

@yzhang93 yzhang93 merged commit 3e966c9 into nod-ai:main Aug 27, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pack_peel_pipeline_matmul and conv_fill_spec_pad consistently fail (on Mac)
3 participants