Skip to content

Conversation

@khushi-411
Copy link
Contributor

As discussed in #93199 (comment).

cc: @Skylion007!

@pytorch-bot pytorch-bot bot added the release notes: cuda release notes category label Jan 30, 2023
@pytorch-bot
Copy link

pytorch-bot bot commented Jan 30, 2023

🔗 Helpful Links

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

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

✅ No Failures

As of commit 06cadeb:
💚 Looks good so far! There are no failures yet. 💚

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

@khushi-411 khushi-411 marked this pull request as ready for review January 30, 2023 16:09
@khushi-411
Copy link
Contributor Author

Thanks for reviewing, @Skylion007!

@pytorchbot label ciflow/trunk

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Jan 30, 2023
@khushi-411
Copy link
Contributor Author

@pytorchmergebot 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

@khushi-411 khushi-411 deleted the use_constexpr branch January 31, 2023 09:48
@malfet
Copy link
Contributor

malfet commented Feb 2, 2023

Hmm, this will break some ancient Windows builds(internally people report ICEs with old CUDA/VC++), but also, I remember there was a warning not to use constexpr with JITerated kernels. @ngimel do you remember the context?

@malfet
Copy link
Contributor

malfet commented Feb 2, 2023

Can we replace those with CONSTEXPR_EXCEPT_WIN_CUDA? (Looks ugly, but fixes the problem)
cc: @Skylion007

@Skylion007
Copy link
Collaborator

Skylion007 commented Feb 2, 2023

@malfet Seems reasonable to me, the constexpr is just a general C++ safety recomendation (making it constexpr allows for more compile time checks etc). Does CONSTEXPR_EXCEPT_WIN_CUDA make it as const on win CUDA? If so, that looks like a good candidate.

@ngimel
Copy link
Collaborator

ngimel commented Feb 2, 2023

Sorry, don't remember, I see that constexpr is used in the jiterator templates, so should be fine.

@seemethere
Copy link
Member

@malfet Seems reasonable to me, the constexpr is just a general C++ safety recomendation (making it constexpr allows for more compile time checks etc). Does CONSTEXPR_EXCEPT_WIN_CUDA make it as const on win CUDA? If so, that looks like a good candidate.

I think this is what we want

malfet added a commit to malfet/pytorch that referenced this pull request Feb 3, 2023
Summary:
Followup after pytorch#93267
Generated by running:
```
for i in *.cu; do sed -i -e "s/constexpr char/CONSTEXPR_EXCEPT_WIN_CUDA char/" $i; done
```

Otherwise, attempts to compile using VS-15.9 results in:
```
D:\pytorch\aten\src\aten\native\cuda\laguerre_polynomial_l.cu(17): fatal error C1001: An internal error has occurred in the compiler.
(compiler file 'msc1.cpp', line 1518)
 To work around this problem, try simplifying or changing the program near the locations listed above.
Please choose the Technical Support command on the Visual C++
 Help menu, or open the Technical Support help file for more information
Internal Compiler Error in D:\VC\Tools\MSVC\14.16.27023\bin\Hostx64\x64\cl.exe.  You will be prompted to send an error report to Microsoft later.
INTERNAL COMPILER ERROR in 'D:\VC\Tools\MSVC\14.16.27023\bin\Hostx64\x64\cl.exe'
    Please choose the Technical Support command on the Visual C++
    Help menu, or open the Technical Support help file for more information

```

Test Plan: CI

Differential Revision: D43011140

fbshipit-source-id: 77df036cd2838ac2d205e5bd1587982cfd730602
@malfet
Copy link
Contributor

malfet commented Feb 3, 2023

Here is the fix for VS2017 compiler exception: #94091

pytorchmergebot pushed a commit that referenced this pull request Feb 4, 2023
Summary:
Followup after #93267
Generated by running:
```
for i in *.cu; do sed -i -e "s/constexpr char/CONSTEXPR_EXCEPT_WIN_CUDA char/" $i; done
```

Otherwise, attempts to compile using VS-15.9 results in:
```
D:\pytorch\aten\src\aten\native\cuda\laguerre_polynomial_l.cu(17): fatal error C1001: An internal error has occurred in the compiler.
(compiler file 'msc1.cpp', line 1518)
 To work around this problem, try simplifying or changing the program near the locations listed above.
Please choose the Technical Support command on the Visual C++
 Help menu, or open the Technical Support help file for more information
Internal Compiler Error in D:\VC\Tools\MSVC\14.16.27023\bin\Hostx64\x64\cl.exe.  You will be prompted to send an error report to Microsoft later.
INTERNAL COMPILER ERROR in 'D:\VC\Tools\MSVC\14.16.27023\bin\Hostx64\x64\cl.exe'
    Please choose the Technical Support command on the Visual C++
    Help menu, or open the Technical Support help file for more information

```

Test Plan: CI

Differential Revision: D43011140

Pull Request resolved: #94091
Approved by: https://github.com/seemethere
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/trunk Trigger trunk jobs on your pull request Merged open source release notes: cuda release notes category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants