[PATCH] D131012: No longer place const volatile global variables in a read only section

2022-08-03 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. On the one hand, we can certainly just have different behavior on BPF targets if this is a common expectation there. On the other hand, if there isn't a requirement to put `const volatile` objects in writable memory, then I'd rather not. If someone has specific

[PATCH] D131012: No longer place const volatile global variables in a read only section

2022-08-03 Thread Andrii Nakryiko via Phabricator via cfe-commits
anakryiko added a comment. In D131012#3697000 , @aaron.ballman wrote: > In D131012#3696810 , @anakryiko > wrote: > >> In D131012#3696327 , >> @aaron.ballman wrote: >>

[PATCH] D131012: No longer place const volatile global variables in a read only section

2022-08-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D131012#3696810 , @anakryiko wrote: > In D131012#3696327 , @aaron.ballman > wrote: > >> In D131012#3695110 , @anakryiko >> wrote: >>

[PATCH] D131012: No longer place const volatile global variables in a read only section

2022-08-03 Thread Andrii Nakryiko via Phabricator via cfe-commits
anakryiko added a comment. In D131012#3696327 , @aaron.ballman wrote: > In D131012#3695110 , @anakryiko > wrote: > >> This will severly break BPF users, as we have a heavy reliance on `const >> volatile`

[PATCH] D131012: No longer place const volatile global variables in a read only section

2022-08-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman planned changes to this revision. aaron.ballman added a comment. In D131012#3695110 , @anakryiko wrote: > This will severly break BPF users, as we have a heavy reliance on `const > volatile` variables being allocated to `.rodata`, which

[PATCH] D131012: No longer place const volatile global variables in a read only section

2022-08-02 Thread Andrii Nakryiko via Phabricator via cfe-commits
anakryiko added a comment. This will severly break BPF users, as we have a heavy reliance on `const volatile` variables being allocated to `.rodata`, which are passed to BPF verifier in Linux kernel as read-only values. This is critical for BPF Complie Once - Run Everywhere approach of writing

[PATCH] D131012: No longer place const volatile global variables in a read only section

2022-08-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 449405. aaron.ballman added a comment. Rebased to get precommit CI to look at it. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131012/new/ https://reviews.llvm.org/D131012 Files: clang/docs/ReleaseNotes.rst

[PATCH] D131012: No longer place const volatile global variables in a read only section

2022-08-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D131012#3694502 , @rjmccall wrote: > I think this is fine for the ABI; the section is generally a > definition-specific property and doesn't affect use sites. > > Do we also need to check for volatile fields of records?

[PATCH] D131012: No longer place const volatile global variables in a read only section

2022-08-02 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. Thanks for the patch. I've asked some of my colleagues who work on libabigail their thoughts on the implications of this change. Due to timezones, it may take some time to

[PATCH] D131012: No longer place const volatile global variables in a read only section

2022-08-02 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. I think this is fine for the ABI; the section is generally a definition-specific property and doesn't affect use sites. Do we also need to check for volatile fields of records? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D131012: No longer place const volatile global variables in a read only section

2022-08-02 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. The code changes look perfectly fine to me, but I'm hopeful someone else has something to say about how acceptable/ABI related this is. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131012/new/

[PATCH] D131012: No longer place const volatile global variables in a read only section

2022-08-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman created this revision. aaron.ballman added reviewers: nickdesaulniers, erichkeane, rjmccall, efriedma. Herald added a project: All. aaron.ballman requested review of this revision. Herald added a project: clang. The C standard hints in a footnote attached to C17 6.7.3p5 that `const`