[PATCH] D79237: [CUDA][HIP] Fix constexpr variables for C++17

This revision was automatically updated to reflect the committed changes. Closed by commit rG049d860707ef: [CUDA][HIP] Fix constexpr variables for C++17 (authored by yaxunl). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D79237: [CUDA][HIP] Fix constexpr variables for C++17

tra accepted this revision. tra added a comment. This revision is now accepted and ready to land. Tested with tensorflow build. The patch- does not seem to break anything now. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79237/new/ https://reviews.llvm.org/D79237 ___

[PATCH] D79237: [CUDA][HIP] Fix constexpr variables for C++17

yaxunl added a comment. ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79237/new/ https://reviews.llvm.org/D79237 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79237: [CUDA][HIP] Fix constexpr variables for C++17

yaxunl updated this revision to Diff 264394. yaxunl marked an inline comment as done. yaxunl added a comment. fix constexpr var in templates CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79237/new/ https://reviews.llvm.org/D79237 Files: clang/include/clang/Sema/Sema.h clang/lib/Sem

[PATCH] D79237: [CUDA][HIP] Fix constexpr variables for C++17

yaxunl marked 2 inline comments as done. yaxunl added inline comments. Comment at: clang/test/SemaCUDA/constexpr-variables.cu:30-31 + static constexpr int c = sizeof(a); + a[0] = &b; + a[1] = &c; + foo(a); tra wrote: > yaxunl wrote: > > tra wrote: > > > Can w

[PATCH] D79237: [CUDA][HIP] Fix constexpr variables for C++17

yaxunl marked an inline comment as done. yaxunl added a comment. In D79237#2039757 , @rsmith wrote: > In D79237#2039559 , @tra wrote: > > > In D79237#2039417 , @tra wrote: >

[PATCH] D79237: [CUDA][HIP] Fix constexpr variables for C++17

rsmith added a comment. In D79237#2039559 , @tra wrote: > In D79237#2039417 , @tra wrote: > > > > > > Bad news -- it breaks the standard C++ library. [...] > build/release+assert+zapcc/bin/../include/c++/v1/uti

[PATCH] D79237: [CUDA][HIP] Fix constexpr variables for C++17

tra added inline comments. Comment at: clang/test/SemaCUDA/constexpr-variables.cu:30-31 + static constexpr int c = sizeof(a); + a[0] = &b; + a[1] = &c; + foo(a); yaxunl wrote: > tra wrote: > > Can we verify the diags for bad cases, too? > By bad cases you mea

[PATCH] D79237: [CUDA][HIP] Fix constexpr variables for C++17

tra added a comment. In D79237#2039417 , @tra wrote: > LGTM in general. Let me check the patch on our tensorflow build. Bad news -- it breaks the standard C++ library. Reproducer: $ bin/clang++ -x cuda /dev/null -fsyntax-only -include algorithm --cu

[PATCH] D79237: [CUDA][HIP] Fix constexpr variables for C++17

yaxunl marked 2 inline comments as done. yaxunl added inline comments. Comment at: clang/test/SemaCUDA/constexpr-variables.cu:30-31 + static constexpr int c = sizeof(a); + a[0] = &b; + a[1] = &c; + foo(a); tra wrote: > Can we verify the diags for bad cases, t

[PATCH] D79237: [CUDA][HIP] Fix constexpr variables for C++17

tra added a comment. LGTM in general. Let me check the patch on our tensorflow build. Comment at: clang/test/SemaCUDA/constexpr-variables.cu:30-31 + static constexpr int c = sizeof(a); + a[0] = &b; + a[1] = &c; + foo(a); Can we verify the diags for bad case

[PATCH] D79237: [CUDA][HIP] Fix constexpr variables for C++17

yaxunl updated this revision to Diff 264329. yaxunl edited the summary of this revision. yaxunl added a comment. add implicit constant attribute to constexpr file scope variables and constexpr static data members in device compilation. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79237

[PATCH] D79237: [CUDA][HIP] Fix constexpr variables for C++17

yaxunl added a comment. In D79237#2031870 , @tra wrote: > > constexpr variables are compile time constants and implicitly const, > > therefore > > they are safe to emit on both device and host side. Besides, in many cases > > they are intended for both

[PATCH] D79237: [CUDA][HIP] Fix constexpr variables for C++17

tra added a comment. > constexpr variables are compile time constants and implicitly const, therefore > they are safe to emit on both device and host side. Besides, in many cases > they are intended for both device and host, therefore it makes sense > to emit them on both device and host sides

[PATCH] D79237: [CUDA][HIP] Fix constexpr variables for C++17

yaxunl added a comment. ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79237/new/ https://reviews.llvm.org/D79237 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79237: [CUDA][HIP] Fix constexpr variables for C++17

yaxunl created this revision. yaxunl added reviewers: tra, rjmccall. constexpr variables are compile time constants and implicitly const, therefore they are safe to emit on both device and host side. Besides, in many cases they are intended for both device and host, therefore it makes sense to emi