[PATCH] D119017: [clang] roll-forward "[clang] Mark `trivial_abi` types as "trivially relocatable"".

devin.jeanpierre added a comment. In D119017#3392961 , @Manna wrote: > LIT test (SemaCXX/attr-trivial-abi.cpp) is failing for x86 build of clang. > The same failures are happening with our downstream X86 clang build. > [...] > The test might need to be u

[PATCH] D119017: [clang] roll-forward "[clang] Mark `trivial_abi` types as "trivially relocatable"".

devin.jeanpierre added a comment. In D119017#3392993 , @Manna wrote: >>> lf so, what target triple, so that I can reproduce? (New to clang >>> development...) > > You can reproduce the test failure if you checkout 32 bit clang compiler. Please bear with

[PATCH] D119017: [clang] roll-forward "[clang] Mark `trivial_abi` types as "trivially relocatable"".

devin.jeanpierre added a comment. Please revert it, I don't have easy access to a Windows machine at work so am struggling with the reproduction instructions. This has failed twice, and this time with a one month delay! Is there any way to run and confirm all tests pass before I try to merge ag

[PATCH] D119017: [clang] roll-forward "[clang] Mark `trivial_abi` types as "trivially relocatable"".

devin.jeanpierre added a comment. In D119017#3400347 , @devin.jeanpierre wrote: > I don't have easy access to a Windows machine at work so am struggling with > the reproduction instructions. Well, after much procrastinating, and an hour of fiddling wit

[PATCH] D123059: re-roll-forward "[clang] Mark `trivial_abi` types as "trivially relocatable"".""

devin.jeanpierre created this revision. devin.jeanpierre added a reviewer: gribozavr2. Herald added subscribers: dexonsmith, pengfei. Herald added a reviewer: aaron.ballman. Herald added a project: All. devin.jeanpierre requested review of this revision. Herald added a project: clang. Herald added

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

devin.jeanpierre added a comment. W! Thank you for submitting, and thanks everyone for the kind and helpful reviews. 😀 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114732/new/ https://reviews.llvm.org/D114732

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

devin.jeanpierre added a comment. Oops, sorry about that. What is the correct way to test/reproduce the change? Do I / can I submit builds to the buildbot manually for testing? Also, should I be rolling back this change, or no? Not sure of the protocol here, this is my first change to Clang. P

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

devin.jeanpierre added a comment. Thank you @gribozavr2 ! I'll hopefully have a roll-forward ready for you either today or tomorrow. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114732/new/ https://reviews.llvm.org/D114732 __

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

devin.jeanpierre added a comment. The core difference of behavior is this, in the logic for setting `canPassInRegisters`: // Clang <= 4 used the pre-C++11 rule, which ignores move operations. // The PS4 platform ABI follows the behavior of Clang 3.2. if (CCK == TargetInfo::CCK_ClangABI4OrP

[PATCH] D119017: [clang] roll-forward "[clang] Mark `trivial_abi` types as "trivially relocatable"".

devin.jeanpierre created this revision. devin.jeanpierre added a reviewer: gribozavr2. Herald added a reviewer: aaron.ballman. Herald added a subscriber: dexonsmith. devin.jeanpierre requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This reve

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

devinj.jeanpierre created this revision. devinj.jeanpierre added reviewers: rsmith, aaron.ballman, rjmccall, ahatanak. Herald added a subscriber: dexonsmith. devinj.jeanpierre requested review of this revision. Herald added a project: clang. This change enables library code to skip paired move-con

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

devinj.jeanpierre added a comment. Just a heads up, I think this is my first change to clang or llvm, and I'd appreciate any feedback you have on the code, review process, etc. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114732/new/ https://revi

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

devin.jeanpierre added a comment. Wow, thanks for the quick response! I really appreciate it. In D114732#3159247 , @Quuxplusone wrote: > - (Major point of contention) To me, `[[trivial_abi]]` does not imply > `[[trivially_relocatable]]` at all. I belie

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

devin.jeanpierre updated this revision to Diff 392214. devin.jeanpierre added a comment. Suggested changes from code review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114732/new/ https://reviews.llvm.org/D114732 Files: clang/docs/LanguageExt

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

devin.jeanpierre added a comment. (Sorry for delayed reply -- I made the mistake of signing up to phabricator with my personal email, which I don't check very well, apparently!) Comment at: clang/include/clang/Basic/AttrDocs.td:3211-3215 +The compiler will pass and return a tr

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

devin.jeanpierre updated this revision to Diff 392606. devin.jeanpierre added a comment. Use `PCK_ARCStrong` to check for ObjC strong pointers, marking them as trivially relocatable as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114732/new/

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

devin.jeanpierre added a comment. (Sorry, I think I'm doing threading wrong here due to lack of experience with phabricator. The reply buttons are grayed out!) > 1. All trivial-abi types are in fact trivially relocated, specifically when > they are passed to functions. > 2. Therefore, all trivi

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

devin.jeanpierre added a comment. In D114732#3178312 , @rjmccall wrote: > Trivial relocation doesn't imply that types have to be safe against being > suddenly relocated during the middle of operations while they're not in a > safe internal state. That

[PATCH] D120086: Explicitly document that `__is_trivially_relocatable(T)` implies that `T` is implicit-lifetime.

devin.jeanpierre created this revision. devin.jeanpierre added reviewers: rsmith, rjmccall. devin.jeanpierre requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Previous change: D114732 The wording here might

[PATCH] D120086: Explicitly document that `__is_trivially_relocatable(T)` implies that `T` is implicit-lifetime.

devin.jeanpierre updated this revision to Diff 409794. devin.jeanpierre added a comment. Rebase off of D119385 . Sorry, I'm bad at git. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120086/new/ https://reviews.llvm

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

devin.jeanpierre updated this revision to Diff 401035. devin.jeanpierre added a comment. Update to pass on Windows (untested right now). On Windows, unlike Itanium ABI, the only special member functions used are the copy constructor and the destructor. If the copy constructor is deleted, then t

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

devin.jeanpierre added a comment. OK, while I'm struggling to set up a new Windows machine so I can make sure this works on Windows... @Quuxplusone, after this is merged, do you want to rebase D67524 on top of this, or should I? I can review it -- I think when

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

devin.jeanpierre updated this revision to Diff 401358. devin.jeanpierre added a comment. Clarify Windows comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114732/new/ https://reviews.llvm.org/D114732 Files: clang/docs/LanguageExtensions.rs

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

devin.jeanpierre added a comment. CI test finished successfully before windows setup did 😢. My workplace's Windows VMs are a bit hosed at the moment... In D114732#3253416 , @Quuxplusone wrote: > In D114732#3253142

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

devin.jeanpierre added a comment. Sorry, I missed your other comments. Let me know if there's anything else I didn't address. Comment at: clang/test/SemaCXX/attr-trivial-abi.cpp:146 +#ifdef _WIN32 +static_assert(!__is_trivially_relocatable(CopyDeleted), ""); +#else ---

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

devin.jeanpierre updated this revision to Diff 401379. devin.jeanpierre added a comment. Fix copy-paste error. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114732/new/ https://reviews.llvm.org/D114732 Files: clang/docs/LanguageExtensions.rst

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

devin.jeanpierre added a comment. In D114732#3256046 , @devin.jeanpierre wrote: > Sorry, I missed your other comments. Let me know if there's anything else I > didn't address. I just noticed that there's a "Done" checkbox next to each comment, and it

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

devin.jeanpierre updated this revision to Diff 401419. devin.jeanpierre added a comment. pull/rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114732/new/ https://reviews.llvm.org/D114732 Files: clang/docs/LanguageExtensions.rst clang/incl

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

devin.jeanpierre added a comment. @rsmith I've pulled+rebased again to avoid the (looks like pre-existing) failure. Copy-pasting the wording on https://llvm.org/docs/MyFirstTypoFix.html#commit-by-proxy: I don’t have commit access, can you land this patch for me? Please use “Devin Jeanpierre je