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

add likely/unlikely macro for unsupport c++20 compiler. #124997

Conversation

xuhancn
Copy link
Collaborator

@xuhancn xuhancn commented Apr 26, 2024

Issue:

Intel validation team found some low version gcc which not support c++20 will occur below issue:

[2024-04-13T08:03:25.142Z] g++ /tmp/torchinductor_root/vd/cvdytwwwlhi63ofh3pwzqfpjga4w4xe7bjfdoavpblbo5khzf3b2.cpp -shared -fPIC -Wall -std=c++17 -Wno-unused-variable -Wno-unknown-pragmas -D_GLIBCXX_USE_CXX11_ABI=0 -I/root/anaconda3/envs/pytorch/lib/python3.8/site-packages/torch/include -I/root/anaconda3/envs/pytorch/lib/python3.8/site-packages/torch/include/torch/csrc/api/include -I/root/anaconda3/envs/pytorch/lib/python3.8/site-packages/torch/include/TH -I/root/anaconda3/envs/pytorch/lib/python3.8/site-packages/torch/include/THC -I/root/anaconda3/envs/pytorch/include/python3.8 -L/root/anaconda3/envs/pytorch/lib/python3.8/site-packages/torch/lib -L/root/anaconda3/envs/pytorch/lib -L/root/anaconda3/envs/pytorch/lib/python3.8/site-packages/torch/lib -ltorch -ltorch_cpu -lgomp -ltorch_python -lc10 -mavx2 -mfma -DCPU_CAPABILITY_AVX2 -O3 -DNDEBUG -ffast-math -fno-finite-math-only -fno-unsafe-math-optimizations -ffp-contract=off -march=native -fopenmp -D C10_USING_CUSTOM_GENERATED_MACROS -o /tmp/torchinductor_root/vd/cvdytwwwlhi63ofh3pwzqfpjga4w4xe7bjfdoavpblbo5khzf3b2.so
[2024-04-13T08:03:25.142Z]
[2024-04-13T08:03:25.142Z] Output:
[2024-04-13T08:03:25.142Z] /tmp/torchinductor_root/vd/cvdytwwwlhi63ofh3pwzqfpjga4w4xe7bjfdoavpblbo5khzf3b2.cpp: In function ‘T parse_arg(PyObject*, size_t) [with T = long int; PyObject = _object; size_t = long unsigned int]’:
[2024-04-13T08:03:25.142Z] /tmp/torchinductor_root/vd/cvdytwwwlhi63ofh3pwzqfpjga4w4xe7bjfdoavpblbo5khzf3b2.cpp:117:10: error: expected identifier before ‘[’ token
[2024-04-13T08:03:25.142Z] [[unlikely]] throw std::runtime_error("expected int arg");
[2024-04-13T08:03:25.142Z] ^

The reason is unlikely need c++20 attribute, ref: https://en.cppreference.com/w/cpp/language/attributes/likely

Solution:

Add MACRO to enable non-c++20 attribute GNU compiler.

cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @peterbell10 @ipiszy @yf225 @chenyang78 @kadeng @muchulee8 @ColinPeppler @amjames @desertfire @chauhang

Copy link

pytorch-bot bot commented Apr 26, 2024

🔗 Helpful Links

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

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

✅ No Failures

As of commit 9e0c46a with merge base b2f521f (image):
💚 Looks good so far! There are no failures yet. 💚

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

@xuhancn xuhancn changed the title add likely/unlikely macro for unsupport compiler. add likely/unlikely macro for unsupport c++20 compiler. Apr 26, 2024
@xuhancn xuhancn requested a review from jgong5 April 26, 2024 08:19
@xuhancn xuhancn added topic: not user facing topic category ciflow/trunk Trigger trunk jobs on your pull request ciflow/binaries_wheel Trigger binary build and upload jobs for wheel on the PR labels Apr 26, 2024
@jgong5 jgong5 requested a review from jansel April 28, 2024 01:59
@DiweiSun
Copy link
Contributor

DiweiSun commented Apr 28, 2024

Functionality Pass with following case:

import torch
from transformers import BertModel

model = BertModel.from_pretrained("bert-base-uncased")
model.eval()

vocab_size = model.config.vocab_size
batch_size = 128
seq_length = 512
data = torch.randint(vocab_size, size=[batch_size, seq_length])
model = torch.compile(model)

with torch.no_grad():
    model(data)

@xuhancn xuhancn marked this pull request as ready for review April 28, 2024 07:00
@xuhancn
Copy link
Collaborator Author

xuhancn commented Apr 28, 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

@xuhancn xuhancn deleted the xu_fix_likely_issue_on_compiler_less_than_std20 branch April 28, 2024 07:03
@xuhancn
Copy link
Collaborator Author

xuhancn commented Apr 28, 2024

@malfet We occurred this issus at pytorch 2.3 test cycle, can you cherry-pick this PR to 2.3 branch for further release?

pytorch-bot bot pushed a commit that referenced this pull request May 3, 2024
# Issue:
Intel validation team found some low version gcc which not support c++20 will occur below issue:
```cmd
[2024-04-13T08:03:25.142Z] g++ /tmp/torchinductor_root/vd/cvdytwwwlhi63ofh3pwzqfpjga4w4xe7bjfdoavpblbo5khzf3b2.cpp -shared -fPIC -Wall -std=c++17 -Wno-unused-variable -Wno-unknown-pragmas -D_GLIBCXX_USE_CXX11_ABI=0 -I/root/anaconda3/envs/pytorch/lib/python3.8/site-packages/torch/include -I/root/anaconda3/envs/pytorch/lib/python3.8/site-packages/torch/include/torch/csrc/api/include -I/root/anaconda3/envs/pytorch/lib/python3.8/site-packages/torch/include/TH -I/root/anaconda3/envs/pytorch/lib/python3.8/site-packages/torch/include/THC -I/root/anaconda3/envs/pytorch/include/python3.8 -L/root/anaconda3/envs/pytorch/lib/python3.8/site-packages/torch/lib -L/root/anaconda3/envs/pytorch/lib -L/root/anaconda3/envs/pytorch/lib/python3.8/site-packages/torch/lib -ltorch -ltorch_cpu -lgomp -ltorch_python -lc10 -mavx2 -mfma -DCPU_CAPABILITY_AVX2 -O3 -DNDEBUG -ffast-math -fno-finite-math-only -fno-unsafe-math-optimizations -ffp-contract=off -march=native -fopenmp -D C10_USING_CUSTOM_GENERATED_MACROS -o /tmp/torchinductor_root/vd/cvdytwwwlhi63ofh3pwzqfpjga4w4xe7bjfdoavpblbo5khzf3b2.so
[2024-04-13T08:03:25.142Z]
[2024-04-13T08:03:25.142Z] Output:
[2024-04-13T08:03:25.142Z] /tmp/torchinductor_root/vd/cvdytwwwlhi63ofh3pwzqfpjga4w4xe7bjfdoavpblbo5khzf3b2.cpp: In function ‘T parse_arg(PyObject*, size_t) [with T = long int; PyObject = _object; size_t = long unsigned int]’:
[2024-04-13T08:03:25.142Z] /tmp/torchinductor_root/vd/cvdytwwwlhi63ofh3pwzqfpjga4w4xe7bjfdoavpblbo5khzf3b2.cpp:117:10: error: expected identifier before ‘[’ token
[2024-04-13T08:03:25.142Z] [[unlikely]] throw std::runtime_error("expected int arg");
[2024-04-13T08:03:25.142Z] ^
```

The season is `unlikely` need c++20 attribute, ref: https://en.cppreference.com/w/cpp/language/attributes/likely

# Solution:
Add MACRO to enable non-c++20 attribute GNU compiler.

Pull Request resolved: #124997
Approved by: https://github.com/jgong5, https://github.com/jansel
pytorchmergebot pushed a commit that referenced this pull request Jun 18, 2024
1. fix some Windows build args.
2. fix c++20 likely issue on Windows, reference: #124997.
3. remove compiler return value check, different compilers return variant value, let's check exception to catch error.

Pull Request resolved: #128765
Approved by: https://github.com/jgong5, https://github.com/jansel
@matthost
Copy link

matthost commented Jul 9, 2024

This is not in the torch 2.3.1 release https://github.com/pytorch/pytorch/blob/v2.3.1/torch/_inductor/codecache.py

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/binaries_wheel Trigger binary build and upload jobs for wheel on the PR ciflow/inductor ciflow/trunk Trigger trunk jobs on your pull request Merged module: inductor open source topic: not user facing topic category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants