8000 Fix recent build error on ppc64le by pratiklp00 · Pull Request #129736 · pytorch/pytorch · GitHub
[go: up one dir, main page]
Skip to content

Fix recent build error on ppc64le #129736

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Fix recent build error on ppc64le #129736

wants to merge 2 commits into from

Conversation

pratiklp00
Copy link
Contributor
@pratiklp00 pratiklp00 commented Jun 28, 2024 •
edited by pytorch-bot bot
Loading

This PR will fix the recent build issue observed on ppc64le.
Fixes #128130

cc @jgong5 @mingfeima @XiaobingSuper @sanchitintel @ashokei @jingxu10

@pytorch-bot PyTorch Bot
Copy link
pytorch-bot bot commented Jun 28, 2024 •
edited
Loading

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/129736

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 360b2b2 with merge base f0da167 (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@pytorch-bot pytorch-bot bot added the module: cpu CPU specific problem (e.g., perf, algorithm) label Jun 28, 2024
@soulitzer soulitzer requested a review from malfet July 2, 2024 12:38
@soulitzer soulitzer added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Jul 2, 2024
albanD
albanD reviewed Jul 2, 2024
View reviewed changes
aten/src/ATen/native/cpu/FlashAttentionKernel.cpp Outdated
@@ -38,6 +38,7 @@ inline void _scale_attn_mask_fusion_kernel(
constexpr int64_t T2_n = 1;
auto vec_scale = at::vec::VectorizedN<T1, T1_n>(val);
int64_t i = 0;
#if !defined(__powerpc__)
Copy link
Collaborator

Choose a reason for hiding this comment

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

won't think make this code silently wrong though?

Copy link
Contributor Author
@pratiklp00 pratiklp00 Jul 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Hi @albanD This is temporary fix.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you make it an error in the else just to make sure we don't have silent correctness issue?

Copy link

Choose a reason for hiding this comment

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

Could you explain why you think this PR will make it silently wrong?

Looks like the change will make PowerPC use only scalar operations and skip the vector operations. The end result is the PPC will just be slower for this function but still correct.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ho sorry, I didn't realized the loop below continues on the same i and not only does the tail. Sorry about that!

albanD
albanD approved these changes Jul 22, 2024
View reviewed changes
Copy link
Collaborator
@albanD albanD left a comment

Choose a reason for hiding this comment

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

Sounds good!

@pratiklp00
Copy link
Contributor Author

Hi @albanD I have committed the new changes which will be the proper fix for the issue #128130 could you please review it. Thanks.

albanD
albanD approved these changes Jul 30, 2024
View reviewed changes
Copy link
Collaborator
@albanD albanD left a comment

Choose a reason for hiding this comment

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

Ho super cool, thanks for taking the time to do this!

@albanD
Copy link
Collaborator
albanD commented Jul 30, 2024

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Jul 30, 2024
@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: This PR needs a release notes: label
If your changes are user facing and intended to be a part of release notes, please use a label starting with release notes:.

If not, please add the topic: not user facing label.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "topic: not user facing"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

Details for Dev Infra team Raised by workflow job

@pratiklp00
Copy link
Contributor Author

@pytorchbot label "topic: not user facing"

@pytorch-bot pytorch-bot bot added the topic: not user facing topic category label Jul 30, 2024
@pratiklp00
Copy link
Contributor Author

Hi @albanD could you please help me with merge

@pratiklp00
Copy link
Contributor Author

@malfet @soulitzer Could you please help me with merge.

@malfet
Copy link
Contributor
malfet commented Aug 1, 2024

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: 1 mandatory check(s) failed. The first few are:

Dig deeper by viewing the failures on hud

Details for Dev Infra team Raised by workflow job

Failing merge rule: Core Maintainers

@pratiklp00
Copy link
Contributor Author

@pytorchbot rebase

@pytorchmergebot
Copy link
Collaborator

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

@pytorchmergebot
Copy link
Collaborator

Successfully rebased FlashAttentionKernel onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout FlashAttentionKernel && git pull --rebase)

@pratiklp00
Copy link
Contributor Author

@albanD @malfet @@soulitzer After rebase there are no CI test failure. Sorry for inconvenience can you please try merging it again.

@malfet
Copy link
Contributor
malfet commented Aug 1, 2024

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@pratiklp00
Copy link
Contributor Author

@malfet @albanD Thanks for the review and help. I would like to know if there is way to push this changes to release/2.4 branch.

@malfet
Copy link
Contributor
malfet commented Aug 6, 2024

@pratiklp00 please read the instructions in #132400 and add appropriate comment if it qualifies a cherry-pick criteria

@pratiklp00
Copy link
Contributor Author

@malfet Thank you.

@pratiklp00
Copy link
Contributor Author

@pytorchbot cherry-pick --onto release/2.4 -c critical

pytorchbot pushed a commit that referenced this pull request Aug 14, 2024
@pratiklp00 @pytorchbot
Fix recent build error on ppc64le (#129736)
3f8277a 
This PR will fix the recent build issue observed on ppc64le.
Fixes #128130

Pull Request resolved: #129736
Approved by: https://github.com/albanD, https://github.com/malfet

(cherry picked from commit 69cbf05)
@pytorchbot pytorchbot mentioned this pull request Aug 14, 2024
Closed
@pytorchbot
Copy link
Collaborator

Cherry picking #129736

The cherry pick PR is at #133416 and it is recommended to link a critical cherry pick PR with an issue. The following tracker issues are updated:

Details for Dev Infra team Raised by workflow job

malfet pushed a commit that referenced this pull request Aug 14, 2024
@pytorchbot @pratiklp00
Fix recent build error on ppc64le (#133416)
58ab993 
Fix recent build error on ppc64le  (#129736)

This PR will fix the recent build issue observed on ppc64le.
Fixes #128130

Pull Request resolved: #129736
Approved by: https://github.com/albanD, https://github.com/malfet

(cherry picked from commit 69cbf05)

Co-authored-by: pratiklp00 <pratikp@linux.ibm.com>
pruthvistony pushed a commit to ROCm/pytorch that referenced this pull request Aug 15, 2024
@pytorchbot @pratiklp00 @pruthvistony
Fix recent build error on ppc64le (pytorch#133416)
bf5d809 
Fix recent build error on ppc64le  (pytorch#129736)

This PR will fix the recent build issue observed on ppc64le.
Fixes pytorch#128130

Pull Request resolved: pytorch#129736
Approved by: https://github.com/albanD, https://github.com/malfet

(cherry picked from commit 69cbf05)

Co-authored-by: pratiklp00 <pratikp@linux.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChipKerchner ChipKerchner ChipKerchner left review comments

@malfet malfet malfet approved these changes

@albanD albanD albanD approved these changes

Assignees
No one assigned
Labels
ciflow/trunk Trigger trunk jobs on your pull request Merged module: cpu CPU specific problem (e.g., perf, algorithm) open source topic: not user facing topic category triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

PyTorch fails to build on ppc64le
7 participants
@pratiklp00 @albanD @pytorchmergebot @malfet @pytorchbot @ChipKerchner @soulitzer
0