[PATCH] D134653: [clang][modules] Over-align the `Module` class

2022-09-26 Thread Jan Svoboda via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG715bd12d2e1d: [clang][modules] Over-align the `Module` class (authored by jansvoboda11). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D134653: [clang][modules] Over-align the `Module` class

2022-09-26 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 added a comment. In D134653#3815703 , @srj wrote: > I opened a question at https://discourse.llvm.org/t/x86-32-bit-testing/65480 > on Friday but got no responses. Is there a better venue for asking the right > people? That's what I would d

[PATCH] D134653: [clang][modules] Over-align the `Module` class

2022-09-26 Thread Steven Johnson via Phabricator via cfe-commits
srj added a comment. > Thanks for verifying the build. You can test for correctness by "building" > the `check-clang` target. Tests are passing locally, LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134653/new/ https://reviews.llvm.org/D1346

[PATCH] D134653: [clang][modules] Over-align the `Module` class

2022-09-26 Thread Steven Johnson via Phabricator via cfe-commits
srj added a comment. > Thanks for verifying the build. You can test for correctness by "building" > the `check-clang` target. Thanks -- I'll also build and test Halide locally. > You can contact the infrastructure working group. Note that we run more > configurations post-commit: https://lab.l

[PATCH] D134653: [clang][modules] Over-align the `Module` class

2022-09-26 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 marked an inline comment as done. jansvoboda11 added a comment. In D134653#3815672 , @srj wrote: > FYI, adding this patch to my local build does allow me to *compile* properly > now. I haven't tested for correctness. Thanks for verifying th

[PATCH] D134653: [clang][modules] Over-align the `Module` class

2022-09-26 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 updated this revision to Diff 462965. jansvoboda11 added a comment. Document reason for `alignas`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134653/new/ https://reviews.llvm.org/D134653 Files: clang/include/clang/Basic/Module.h

[PATCH] D134653: [clang][modules] Over-align the `Module` class

2022-09-26 Thread Steven Johnson via Phabricator via cfe-commits
srj added a comment. In D134653#3815627 , @jansvoboda11 wrote: > Pre-commit CI only runs on x64 Debian and x64 Windows. I don't have good way > to reproduce the issue, so ideally the person who reported it (@ms178?) steps > up. FYI, adding this patch

[PATCH] D134653: [clang][modules] Over-align the `Module` class

2022-09-26 Thread Steven Johnson via Phabricator via cfe-commits
srj added a comment. In D134653#3815627 , @jansvoboda11 wrote: > Pre-commit CI only runs on x64 Debian and x64 Windows. I don't have good way > to reproduce the issue, so ideally the person who reported it (@ms178?) steps > up. I'm the one who reporte

[PATCH] D134653: [clang][modules] Over-align the `Module` class

2022-09-26 Thread Steven Johnson via Phabricator via cfe-commits
srj added inline comments. Comment at: clang/include/clang/Basic/Module.h:96 /// Describes a module or submodule. -class Module { +class alignas(8) Module { public: Probably worth a comment here, it's not necessarily obvious to the reader that this is to fix a

[PATCH] D134653: [clang][modules] Over-align the `Module` class

2022-09-26 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 added a subscriber: ms178. jansvoboda11 added a comment. Pre-commit CI only runs on x64 Debian and x64 Windows. I don't have good way to reproduce the issue, so ideally the person who reported it (@ms178?) steps up. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D134653: [clang][modules] Over-align the `Module` class

2022-09-26 Thread Steven Johnson via Phabricator via cfe-commits
srj added a comment. How did the original change pass presubmit testing -- is LLVM no longer testing on 32-bit targets? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134653/new/ https://reviews.llvm.org/D134653 ___

[PATCH] D134653: [clang][modules] Over-align the `Module` class

2022-09-26 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 created this revision. jansvoboda11 added reviewers: srj, mgorny, Bigcheese. Herald added a subscriber: ributzka. Herald added a project: All. jansvoboda11 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This makes `llvm::Point