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

[Deprecated][inductor] switch CppCodeCache to new cpp_builder. #128303

Closed
wants to merge 14 commits into from

Conversation

xuhancn
Copy link
Collaborator

@xuhancn xuhancn commented Jun 9, 2024

This PR is the implemention of #124245 step 2.1

  1. Switch CppCodeCache to new CPP builder.
  2. Switch AotCodeCompiler to new CPP builder.
  3. Switch HalideCodeCache to new CPP builder.

TODO:
Current still need to optimize code structure and circle import. I will do this work in further PR. I have did the exploration: #128330

cc @peterjc123 @mszhanyi @skyline75489 @nbcsm @vladimir-aubrecht @iremyux @Blackhex @cristianPanaite @jgong5 @mingfeima @XiaobingSuper @sanchitintel @ashokei @jingxu10 @voznesenskym @penguinwu @EikanWang @Guobing-Chen @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @peterbell10 @ipiszy @yf225 @chenyang78 @kadeng @muchulee8 @ColinPeppler @amjames @desertfire @chauhang

Copy link

pytorch-bot bot commented Jun 9, 2024

🔗 Helpful Links

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

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

✅ You can merge normally! (15 Unrelated Failures)

As of commit 033835f with merge base 5fba5d8 (image):

FLAKY - The following jobs failed but were likely due to flakiness present on trunk:

BROKEN TRUNK - The following jobs failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

UNSTABLE - The following jobs failed but were likely due to flakiness present on trunk and has been marked as unstable:

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

@xuhancn xuhancn added ciflow/trunk Trigger trunk jobs on your pull request module: windows Windows support for PyTorch intel priority matters to intel architecture from performance wise intel This tag is for PR from Intel topic: not user facing topic category labels Jun 9, 2024
@xuhancn
Copy link
Collaborator Author

xuhancn commented Jun 9, 2024

@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

Tried to rebase and push PR #128303, but it was already up to date. Try rebasing against main by issuing:
@pytorchbot rebase -b main

@xuhancn
Copy link
Collaborator Author

xuhancn commented Jun 9, 2024

@pytorchbot rebase -b main

@pytorchmergebot
Copy link
Collaborator

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

@pytorchmergebot
Copy link
Collaborator

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

@xuhancn xuhancn force-pushed the xu_switch_to_new_cpp_builder branch 4 times, most recently from 8504f7f to 80d49be Compare June 9, 2024 13:40
@xuhancn xuhancn force-pushed the xu_switch_to_new_cpp_builder branch from 246ffb1 to 891cf9e Compare June 9, 2024 15:32
@xuhancn xuhancn requested review from jgong5 and desertfire June 9, 2024 16:53
@xuhancn xuhancn marked this pull request as ready for review June 9, 2024 16:53
@bdhirsh bdhirsh added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Jun 10, 2024
@desertfire
Copy link
Contributor

internal-external out of sync. Will need to wait for a while.

@facebook-github-bot
Copy link
Contributor

@desertfire has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@xuhancn
Copy link
Collaborator Author

xuhancn commented Jun 14, 2024

Hi @desertfire , could you please let me know the import status?

@xuhancn xuhancn changed the title switch CppCodeCache to new cpp_builder. [inductor] switch CppCodeCache to new cpp_builder. Jun 16, 2024
jgong5
jgong5 previously requested changes Jun 18, 2024
Copy link
Collaborator

@jgong5 jgong5 left a comment

Choose a reason for hiding this comment

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

Can you remove the unused or commented out code?

torch/_inductor/codecache.py Show resolved Hide resolved
@xuhancn xuhancn dismissed jgong5’s stale review June 18, 2024 09:38

restructure code to solve it in next PR, let's land it firstly.

@xuhancn xuhancn requested a review from jgong5 June 18, 2024 09:38
pytorchmergebot pushed a commit that referenced this pull request Jun 19, 2024
…129059)

I have run the first inductor case on Windows base on the exploration code: #128330
Due to some fundamental PR still need pass `fb_code`: #128303
This PR would land some part of exploration code:
1. Fix Windows python module ext type: pyd.
2. Add function export declaration for Windows.

Pull Request resolved: #129059
Approved by: https://github.com/jgong5, https://github.com/jansel
@facebook-github-bot
Copy link
Contributor

@desertfire has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@xuhancn
Copy link
Collaborator Author

xuhancn commented Jun 20, 2024

@desertfire Please help on validate new two fix in fb_code:

  1. 'CUDA_HOME environment variable is not set.
  2. error: no such file or directory: 'cd7r7ustk6uz5y6ctrjm4iuakbwa6ov7qiohl7qzqvx66nztzu2a.cpp'

@xuhancn
Copy link
Collaborator Author

xuhancn commented Jun 20, 2024

@pytorchmergebot rebase -b main

@pytorchmergebot
Copy link
Collaborator

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

@pytorchmergebot
Copy link
Collaborator

Rebase failed due to Command git -C /home/runner/work/pytorch/pytorch rebase refs/remotes/origin/main pull/128303/head returned non-zero exit code 1

Rebasing (1/10)
Rebasing (2/10)
Rebasing (3/10)
Rebasing (4/10)
Auto-merging torch/_inductor/codecache.py
Auto-merging torch/_inductor/cpp_builder.py
CONFLICT (content): Merge conflict in torch/_inductor/cpp_builder.py
error: could not apply 891cf9e1b9... switch AotCodeCompiler library linker to new cpp_builder.
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Could not apply 891cf9e1b9... switch AotCodeCompiler library linker to new cpp_builder.

Raised by https://github.com/pytorch/pytorch/actions/runs/9599403273

@xuhancn
Copy link
Collaborator Author

xuhancn commented Jun 20, 2024

@desertfire this PR seems damaged, please move to new one: #129144

pytorchmergebot pushed a commit that referenced this pull request Jun 27, 2024
Original PRs is damaged by confilct and rebase: #128303, #129144

This PR just switch `HalideCodeCache` to new cpp_builder and it is not `fb_code` related. It can merge without `fb_code` test.
Let's land this change firstly.

Pull Request resolved: #129441
Approved by: https://github.com/jgong5, https://github.com/desertfire
@xuhancn xuhancn changed the title [inductor] switch CppCodeCache to new cpp_builder. [Deprecated][inductor] switch CppCodeCache to new cpp_builder. Jun 30, 2024
@xuhancn xuhancn closed this Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/inductor ciflow/trunk Trigger trunk jobs on your pull request intel priority matters to intel architecture from performance wise intel This tag is for PR from Intel module: inductor module: windows Windows support for PyTorch 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
Development

Successfully merging this pull request may close these issues.

None yet

7 participants