[PATCH] D102015: [clang CodeGen] Don't crash on large atomic function parameter.

2021-05-17 Thread Eli Friedman via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG698568b74c93: [clang CodeGen] Don't crash on large atomic function parameter. (authored by efriedma). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102015/ne

[PATCH] D102015: [clang CodeGen] Don't crash on large atomic function parameter.

2021-05-14 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Thanks, LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102015/new/ https://reviews.llvm.org/D102015 ___

[PATCH] D102015: [clang CodeGen] Don't crash on large atomic function parameter.

2021-05-14 Thread Eli Friedman via Phabricator via cfe-commits
efriedma updated this revision to Diff 345543. efriedma added a comment. Figured out a way to test the EmitDelegateCallArg codepath. (A thunk doesn't work because the code is guarded by `!CurFuncIsThunk`.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D102015: [clang CodeGen] Don't crash on large atomic function parameter.

2021-05-10 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In D102015#2748441 , @efriedma wrote: > In D102015#2743634 , @rjmccall > wrote: > >> Objective-C object types also have aggregate evaluation kind. Those can >> only be used as value ty

[PATCH] D102015: [clang CodeGen] Don't crash on large atomic function parameter.

2021-05-10 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. > Using _Atomic for C structs with non-trivially destructible fields currently > doesn't work, so maybe that should just be disallowed. I'd prefer to disallow _Atomic on all types that aren't trivially destructible, yes. Not impossible to support, of course, but I don

[PATCH] D102015: [clang CodeGen] Don't crash on large atomic function parameter.

2021-05-10 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. Using `_Atomic` for C structs with non-trivially destructible fields currently doesn't work, so maybe that should just be disallowed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102015/new/ https://reviews.llvm.org/D102

[PATCH] D102015: [clang CodeGen] Don't crash on large atomic function parameter.

2021-05-10 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. C structs with ObjC pointer fields under ARC are non-trivial to destruct too. struct foo { int big[128]; id a; }; void test_atomic_array_param(_Atomic(struct foo) a) { } Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D102015: [clang CodeGen] Don't crash on large atomic function parameter.

2021-05-10 Thread Eli Friedman via Phabricator via cfe-commits
efriedma updated this revision to Diff 344193. efriedma added a comment. Use isRecordType() instead of checking for an atomic type. Fix the caller side as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102015/new/ https://reviews.llvm.org/D1

[PATCH] D102015: [clang CodeGen] Don't crash on large atomic function parameter.

2021-05-10 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. In D102015#2748441 , @efriedma wrote: >> ...I'm confused about why this code is doing what it's doing with cleanups, >> though. Why does it only apply when the parameter is indirect? I believe >> `isParamDestroyedInCallee()`

[PATCH] D102015: [clang CodeGen] Don't crash on large atomic function parameter.

2021-05-10 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. In D102015#2743634 , @rjmccall wrote: > Objective-C object types also have aggregate evaluation kind. Those can only > be used as value types in an old, fragile ObjC ABI, but I believe we haven't > yet formally decided to remo

[PATCH] D102015: [clang CodeGen] Don't crash on large atomic function parameter.

2021-05-06 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Objective-C object types also have aggregate evaluation kind. Those can only be used as value types in an old, fragile ObjC ABI, but I believe we haven't yet formally decided to remove support for that. Fortunately, however, there's a much better simplification avail

[PATCH] D102015: [clang CodeGen] Don't crash on large atomic function parameter.

2021-05-06 Thread Eli Friedman via Phabricator via cfe-commits
efriedma created this revision. efriedma added reviewers: rsmith, ahatanak, rjmccall. Herald added a subscriber: jfb. efriedma requested review of this revision. Herald added a project: clang. I wouldn't recommend writing code like the testcase; a function parameter isn't atomic, so using an atom