[PATCH] D79744: clang: Use byref for aggregate kernel arguments

2020-08-06 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm closed this revision. arsenm added a comment. 30eeb742f1d11d7a7036e3b8a3bffc1dfd252082 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79744/new/ https://reviews.llvm.org/D79744

[PATCH] D79744: clang: Use byref for aggregate kernel arguments

2020-08-06 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. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79744/new/ https://reviews.llvm.org/D79744 ___ cfe-commits mailing list cf

[PATCH] D79744: clang: Use byref for aggregate kernel arguments

2020-08-05 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm updated this revision to Diff 283345. arsenm added a comment. Reword comment CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79744/new/ https://reviews.llvm.org/D79744 Files: clang/include/clang/CodeGen/CGFunctionInfo.h clang/lib/CodeGen/CGCall.cpp clang/lib/CodeGen/TargetIn

[PATCH] D79744: clang: Use byref for aggregate kernel arguments

2020-08-05 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:8816 + // FIXME: Should use byref when promoting pointers in structs, but this + // requires adding implementing the coercion. + if (!getContext().getLangOpts().OpenCL && LTy == OrigLTy && ---

[PATCH] D79744: clang: Use byref for aggregate kernel arguments

2020-08-05 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:8816 + // FIXME: Should use byref when promoting pointers in structs, but this + // requires adding implementing the coercion. + if (!getContext().getLangOpts().OpenCL && LTy == OrigLTy && -

[PATCH] D79744: clang: Use byref for aggregate kernel arguments

2020-08-05 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:1997 case ABIArgInfo::Ignore: + case ABIArgInfo::IndirectAliased: return false; arsenm wrote: > arsenm wrote: > > rjmccall wrote: > > > arsenm wrote: > > > > rjmccall wrote: > >

[PATCH] D79744: clang: Use byref for aggregate kernel arguments

2020-08-05 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:1997 case ABIArgInfo::Ignore: + case ABIArgInfo::IndirectAliased: return false; arsenm wrote: > rjmccall wrote: > > arsenm wrote: > > > rjmccall wrote: > > > > In principle, this

[PATCH] D79744: clang: Use byref for aggregate kernel arguments

2020-08-04 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:1997 case ABIArgInfo::Ignore: + case ABIArgInfo::IndirectAliased: return false; rjmccall wrote: > arsenm wrote: > > rjmccall wrote: > > > In principle, this can be `inreg` just as

[PATCH] D79744: clang: Use byref for aggregate kernel arguments

2020-08-03 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:1997 case ABIArgInfo::Ignore: + case ABIArgInfo::IndirectAliased: return false; arsenm wrote: > rjmccall wrote: > > In principle, this can be `inreg` just as much as Indirect ca

[PATCH] D79744: clang: Use byref for aggregate kernel arguments

2020-07-29 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm updated this revision to Diff 281629. arsenm marked 5 inline comments as done. arsenm added a comment. Address comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79744/new/ https://reviews.llvm.org/D79744 Files: clang/include/clang/CodeGen/CGFunctionInfo.h clang/lib/Code

[PATCH] D79744: clang: Use byref for aggregate kernel arguments

2020-07-29 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:1997 case ABIArgInfo::Ignore: + case ABIArgInfo::IndirectAliased: return false; rjmccall wrote: > In principle, this can be `inreg` just as much as Indirect can. The IR verifier c

[PATCH] D79744: clang: Use byref for aggregate kernel arguments

2020-07-24 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: clang/include/clang/CodeGen/CGFunctionInfo.h:52 +/// IndirectAliased - Similar to Indirect, but the pointer may not be +/// writable. +IndirectAliased, Hmm. I guess there's actually two different potential

[PATCH] D79744: clang: Use byref for aggregate kernel arguments

2020-07-24 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm updated this revision to Diff 280607. arsenm added a comment. Use distinct ABIArgInfo::Kind. Also don't enable this for OpenCL yet, since that requires fixing the callable kernel workaround CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79744/new/ https://reviews.llvm.org/D79744

[PATCH] D79744: clang: Use byref for aggregate kernel arguments

2020-07-23 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm added a comment. In D79744#2165929 , @rjmccall wrote: > Arguably we should add this attribute to all indirect arguments. I can > understand not wanting to update all the test cases, but you could probably > avoid adding a new IndirectByRef kind o

[PATCH] D79744: clang: Use byref for aggregate kernel arguments

2020-07-21 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Arguably we should add this attribute to all indirect arguments. I can understand not wanting to update all the test cases, but you could probably avoid adding a new IndirectByRef kind of ABIArgInfo by treating kernels specially in ConstructAttributeList. Or, sorry,